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

fix: Ensure referential stability for values #617

Merged
merged 4 commits into from
Sep 2, 2024

Conversation

franky47
Copy link
Member

@franky47 franky47 commented Aug 28, 2024

Before this PR, any update of a search param (via nuqs or otherwise) would trigger all the parsers to run, causing unstable references for non-primitive states (objects, arrays, maybe even Dates?).

This caused effects to re-run needlessly, and it is wasteful: we already have the values, and other search params which haven't changed don't need to be parsed again.

This PR implements a cache of the serialized query string value along with the cache of the internal state (using React refs), and checks for a change before calling the parsers if needed.

Related tweet: https://x.com/fortysevenfx/status/1828786748354408489

Caveats

This is an edge case, but having two different parsers on the same key (eg: parseAsInteger and parseAsFloat) will result in the latest updated value being propagated to all the hooks registered on the same key.

Example: if setting foo to 1.234 via parseAsFloat, a hook listening to foo with parseAsInteger will have a state of 1.234, not 1. Before this PR, the state would flash first with 1.234, then the parser would run and correct it to 1.

Anyway, this is not a recommended use of the library (one key, one parser, then derive state if needed).

Copy link

vercel bot commented Aug 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nuqs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 2, 2024 6:29pm

@franky47 franky47 changed the title test: Making sure it fails beforehand fix: Ensure referential equality for values Aug 28, 2024
@franky47 franky47 changed the title fix: Ensure referential equality for values fix: Ensure referential stability for values Aug 28, 2024
@franky47 franky47 marked this pull request as ready for review August 28, 2024 14:46
Copy link

pkg-pr-new bot commented Sep 2, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/nuqs@617

commit: 88d2c5e

@franky47 franky47 merged commit 6f22280 into next Sep 2, 2024
18 checks passed
@franky47 franky47 deleted the fix/referential-stability branch September 2, 2024 19:24
Copy link

github-actions bot commented Sep 2, 2024

🎉 This PR is included in version 1.19.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link

github-actions bot commented Sep 2, 2024

🎉 This PR is included in version 1.19.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

1 participant