Feat: memory location search hook

[mitulagr2]

Return searchHook from memoryLocation. This can be passed to Router.

fixes #447

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (48b63a9) to head (d6a84d0).
Report is 23 commits behind head on v3.

Additional details and impacted files

@@            Coverage Diff            @@
##                v3      #497   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            1         1           
=========================================
  Hits             1         1           

Flag	Coverage Δ
	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tillprochaska]

Hi @mitulagr2, thanks for taking the time to fix this. I’m currently working on an app that makes heavy use of URL search parameters to encode/share UI state, so I’m quite interested in better testing support for the useSearch hook.

I’ve tested your changes and noticed a few issues:

• The location hook includes the search string in the returned value. The default browser location hook returns the path only.
• The search hook returns a single-valued array, whereas the browser location search hook returns a string.

I’ve tried to fix these issues here: https://github.com/tillprochaska/wouter/tree/memory-location-search-hook

@molefrog I’ve seen that this PR has been open for quite a while. Is there anything else I could help with to move this forward?

I saw that the recent release includes a new useSearchParams hook. It would probably be nice if the memory location implementation had support for that as well, but that could probably be added separately on top of these changes.

[molefrog]

I saw that the recent release includes a new useSearchParams hook. It would probably be nice if the memory location implementation had support for that as well, but that could probably be added separately on top of these changes.

useSearchParams uses useSearch internal so it should work automatically with any location hook.

[molefrog]

Sadly, I started reviewing this PR a while a ago and I didn't press send.

[tillprochaska]

@molefrog Thanks for the update!

@mitulagr2 In case this PR isn’t relevant to you anymore or you don’t have time, feel free to let me know and I’m happy to pick up the PR and apply the requested changes :)

[mitulagr2]

Thanks for the review! I'll push the changes by tomorrow

[mitulagr2]

@molefrog @tillprochaska Could you please let me know if there's anything else that needs to be changed?

Currently, the memory location hook does not throw any error if both, explicit search parameter and path with search query are used simultaneously

const { hook, searchHook } = memoryLocation({ path: '/foo?key=value' });
<Router hook={hook} searchHook={searchHook}>

// or more explicit
const { hook, searchHook } = memoryLocation({ path: '/foo', searchPath: 'key=value' });

// also valid
const { hook, searchHook } = memoryLocation({ path: '/foo?foo=bar', searchPath: 'key=value' });

[tillprochaska]

Hi @mitulagr2, that’s great, thank you!

I think there are still two small issues where there memory location implementation would be different from the default browser location implementation (see comments below).

[mitulagr2]

Thank you for the detail! I've made said changes.

[molefrog]

LGTM!

[molefrog]

Available in the latest release!

[tillprochaska]

Thank you both, much appreciated!