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

update a.k.a setter function is not stable in v2 #852

Closed
SaveliiLukash opened this issue Jan 8, 2025 · 4 comments · Fixed by #841
Closed

update a.k.a setter function is not stable in v2 #852

SaveliiLukash opened this issue Jan 8, 2025 · 4 comments · Fixed by #841
Labels
bug Something isn't working

Comments

@SaveliiLukash
Copy link

Context

What's your version of nuqs?

"nuqs": "2.3.0"

What framework are you using?

✅ Next.js (app router)

Which version of your framework are you using?

"next": "15.1.4",
"react": "^19.0.0",
"react-dom": "^19.0.0"

Description

nuqs update function a.k.a setter in useQueryState(s) is no longer stable (by reference) in v2 in contrast to v1

v1 perfectly aligns with react's own useState the setState of which is guaranteed to be stable
v2 breaks this behavior

The following useEffect will run indefinitely in a loop in v2, but not in v1:

  const [,setUsername] = useQueryState("username")

  useEffect(() => {
    console.log("Running setUsername")
    setUsername(null)
  }, [setUsername]);

Reproduction

I set up a simple next.js starter repo with nuqs and hosted it on Vercel:

V2: GitHub v2 | Vercel v2
v1: GitHub v1 | Vercel v1

  1. Open the page
  2. Note the console output Running setUsername printed in an infinite loop to console in v2
  3. Note the console output Running setUsername printed once only in v1
  4. The app crashes in Firefox after some time due to History API overload (not on video, but reproducable)

Demo:

CleanShot.2025-01-08.at.21.23.52.mp4

The point of interest is NUQS component.

Research

The update function in v1 had the following dependency array, which works just fine:

[key, history, shallow, scroll, throttleMs, startTransition]

The v2 has introduced adapter to the dependency array:

[key, history, shallow, scroll, throttleMs, startTransition, adapter]

adapter is a context value which is a composite object:

return {
searchParams: optimisticSearchParams,
updateUrl,
// See: https://github.com/47ng/nuqs/issues/603#issuecomment-2317057128
rateLimitFactor: 2
}

In this context updateUrl is stable locally thanks to an empty dependency array ([]) of useCallback

However the object that is return'ed is dynamic (recreated on every render)

return {
  searchParams: optimisticSearchParams,
  updateUrl,
  // See: https://github.com/47ng/nuqs/issues/603#issuecomment-2317057128
  rateLimitFactor: 2
}

Simplified React Example

Current context provider for nuqs adapters can be simplified to the following snippet, where compositeObjectValue, used by context consumers will not be stable:

function MyProvider({ children }) {
  const [value, setValue] = useState(0);

  // Use memo prevents object creation on every render
  // But it is still recreated on every `value` change
  const compositeObjectValue = useMemo(() => ({ value, setValue }), [value, setValue]);

  return (
    <MyContext.Provider value={compositeObjectValue}>
      {children}
    </MyContext.Provider>
  );
}

compositeObjectValue here represents adapter:

[key, history, shallow, scroll, throttleMs, startTransition, adapter]

Possible solutions

  1. Update the dependency array of update function to rely on stable object values instead of a dynamic object itself (if possible):
- [key, history, shallow, scroll, throttleMs, startTransition, adapter]
+ [key, history, shallow, scroll, throttleMs, startTransition, adapter.updateUrl, adapter.rateLimitFactor]
  1. Another common practice is splitting a context in two, for dynamic and stable values:
const ValueContext = createContext();
const SetterContext = createContext();

function MyProvider({ children }) {
  const [value, setValue] = useState(0);

  return (
    <ValueContext.Provider value={value}>
      <SetterContext.Provider value={setValue}>
        {children}
      </SetterContext.Provider>
    </ValueContext.Provider>
  );
}
  1. If not possible to restore the v1 behavior, update the docs to reflect the breaking change and warn users not to use update function in dependency arrays.
    It also has to be stated that useQueryState's update does not align with useState's setState in terms of stability

Extra thoughts

From our side (as consumers of this library) a solution is to remove update functions of useQueryState(s) from all dependency arrays. It works just fine and having it specified is, probably, redundant in the first place.

However, placing a setter function in a dependency array is not a violation in any case and is perfectly fine according to React.

Eslit's react-hooks/exhaustive-deps highlights the need to add an update function to the dependency array, so that is what many devs might just do.

P.S. @franky47 Thanks for your work on this amazing library which is really helpful and is a pleasure to use! Looking forward to many years of collaboration!

@SaveliiLukash SaveliiLukash added the bug Something isn't working label Jan 8, 2025
@franky47
Copy link
Member

franky47 commented Jan 8, 2025

Thanks for the detailed report! I fixed this in #841, could you give [email protected] a try and let me know if it works better? It also solves some unnecessary re-renders (see #849).

@SaveliiLukash
Copy link
Author

Awesome! Tested this version both in demo examples and in our project which uses nuqs and can confirm that [email protected] fixes the issue! There is also a lot less renders which is nice.

We'll use 2.3.1-beta.2 for know and waiting for stable 2.3.1 release!

Thanks for your quick response and your work in general!

@franky47 franky47 linked a pull request Jan 8, 2025 that will close this issue
@franky47
Copy link
Member

franky47 commented Jan 8, 2025

Good, let me know if you hit any snags with the beta, I'm planning on shipping 2.3.1 tomorrow.

@franky47 franky47 closed this as completed Jan 8, 2025
@franky47
Copy link
Member

[email protected] was released (somehow the Semantic Release bot didn't make the connection with this issue).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants