Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: useSearchParams utility hook #391

Open
wants to merge 4 commits into
base: v3
Choose a base branch
from

Conversation

junwen-k
Copy link

@junwen-k junwen-k commented Nov 30, 2023

This PR introduce a useSearchParams utility hook that is similar to React-Router's useSearchParams hook.

Due to the minimalistic approach of this library, the utility hook is designed to be as simple as possible while still being useful for most cases. We could even remove the callback setState style if want to be truly minimalist. It does not support "default" search params, does not support array values out of the box to keep the API surface minimal. Let me know what do you think :)

It respects useSearch and useLocation and uses them internally for the utility hook.

This is based on my understanding with this library, feel free to give any feedbacks :)

Closes #368

Copy link

codesandbox bot commented Nov 30, 2023

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

stackblitz bot commented Nov 30, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

codecov bot commented Nov 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dd372ac) 100.00% compared to head (972b05b) 100.00%.
Report is 10 commits behind head on v3.

Additional details and impacted files
@@            Coverage Diff            @@
##                v3      #391   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            1         1           
=========================================
  Hits             1         1           

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

@junwen-k
Copy link
Author

I have yet to include any documentation in the README though. But feel free to take a look at the implementation details first before we proceed to documentation.

Comment on lines 41 to 60
export const useSearchParams = ({ ssrSearch = "" } = {}) => {
const search = useSearch({ ssrSearch });
const searchParamsRef = useRef(new URLSearchParams(search));
searchParamsRef.current = useMemo(
() => new URLSearchParams(search),
[search]
);

const setSearchParams = useCallback((nextInit, navOpts) => {
const newSearchParams = new URLSearchParams(
typeof nextInit === "function"
? nextInit(searchParamsRef.current)
: nextInit
);
navigate("?" + newSearchParams, navOpts);
}, []);

return [searchParamsRef.current, setSearchParams];
};

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some ideas to reduce extra React imports and make this function smaller:

Suggested change
export const useSearchParams = ({ ssrSearch = "" } = {}) => {
const search = useSearch({ ssrSearch });
const searchParamsRef = useRef(new URLSearchParams(search));
searchParamsRef.current = useMemo(
() => new URLSearchParams(search),
[search]
);
const setSearchParams = useCallback((nextInit, navOpts) => {
const newSearchParams = new URLSearchParams(
typeof nextInit === "function"
? nextInit(searchParamsRef.current)
: nextInit
);
navigate("?" + newSearchParams, navOpts);
}, []);
return [searchParamsRef.current, setSearchParams];
};
export const useSearchParams = ({ ssrSearch = "" } = {}) => {
const search = useSearch({ ssrSearch });
const params = useMemo(() => new URLSearchParams(search)));
const setSearchParams = useEvent((nextInit, navOpts) => {
const newSearchParams = new URLSearchParams(
typeof nextInit === "function"
? nextInit(params)
: nextInit
);
navigate("?" + newSearchParams, navOpts);
});
return [params, setSearchParams];
};

Here, I replaced useCallback with useEvent. We should not see degraded performance, unless the component that uses it renders too often, e.g. once every 50-300 ms (but that isn't really our responsibility, since when used standalone this hook will re-render only when the query string changes, which in real-life scenarios shouldn't be too often). Plus, useEvent is already part of react-deps.js.

I'm still trying to figure out how useMemo can be avoided further on. The question is whether we really want to memoise it. Considering, that the hook doesn't render too often, it should be safe to always return a new instance of URLSearchParams. Why:

  1. Immutability. Is it safe to re-use this object? What if developer decides to mutate it for some reason, this would cause some undefined bevahiour.
  2. The memory/CPU footprint of URLSearchParams. I'm not sure how browsers implement it, but at the first glance it doesn't seem like a heavy resource. Though, I will make a proper research to figure this out.
  3. Stability. One of the caveats could be that we always return a new object reference. This could harm performance only if searchParams instance is further passed down to components as a prop. But in reality, why would someone want to do that? I assume that the normal use-case is the get the object, and then extract the required primitive objects from it, e.g. searchParams.get("foo")

Anyways, there is a trade-off that we should evaluate there.

@junwen-k
Copy link
Author

I think I will make the changes required after resolving #393 first.
I might also remove the useMemo as well.

Thanks @molefrog

@junwen-k
Copy link
Author

junwen-k commented Apr 4, 2024

@molefrog Hi, any chance to see searchParams officially supported in this library? Related PR #422 :)

@MatejFacko
Copy link

Hello guys, any update on this? we would love to use this hook :)

@theguacamoleking
Copy link

theguacamoleking commented Sep 13, 2024

Hello! Lurker here with some info that might be of use.

I tried implementing something similar to this in the past where search param changes cause a navigation event, triggering a rerender from which the new state can be inferred from the history API. While this worked on Chrome, I found that Safari will throw an error if more than 100 history pushes occur within 30s

SecurityError: Attempt to use history.pushState() more than 100 times per 10 seconds

You can trigger this pretty easily in the console

setInterval(() => history.pushState(undefined, undefined, location.href.replace(/\?.*/, "") + `?test=${Math.random()}`),10)

So yeah, just a heads up as this limitation isn't super well known and can fundamentally break an application in cases where application state is coupled to query params.

From what I can tell, Wouter is using the history API as the main source of truth for state - I assume other libraries avoid this issue by decoupling application state from the history API (and debouncing history pushes)?

@not-first
Copy link

Hey, I was wondering if there was any updates whether this would be included in the library. I would love to use the hook!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useSearchParams for extracting and modifying search parameters
5 participants