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

useQueryState throttled option causes state to reset when component rerenders #726

Closed
gijsroge opened this issue Oct 30, 2024 · 8 comments
Closed
Labels
adapters/remix Uses the Remix adapter bug Something isn't working

Comments

@gijsroge
Copy link

gijsroge commented Oct 30, 2024

Context

What's your version of nuqs?

2.1.0

What framework are you using?

  • ✅ Remix

Which version of your framework are you using?

2.13.1

Description

Whenever the server finishes a request the component get's re-rendered and the useQueryState returns an old search state.

useQueryStates does not have this issue, seems to only be happening with useQueryState

If I read the docs I would assume this should not be the case https://nuqs.47ng.com/docs/options#throttling-url-updates

the state returned by the hook is always updated instantly, to keep UI responsive. Only changes to the URL, and server requests when using shallow: false, are throttled.

Reproduction

https://stackblitz.com/~/github.com/gijsroge/nuqs-throttle-issue?file=app/routes/_index.tsx

If you type in the search box you will notice the issue.

This issue is only apparent if you use the throttle feature.

@gijsroge gijsroge added the bug Something isn't working label Oct 30, 2024
@franky47 franky47 added the adapters/remix Uses the Remix adapter label Oct 30, 2024
@gijsroge gijsroge changed the title Throttled state is reset when component rerenders useQueryState throttled option causes state to reset when component rerenders Oct 30, 2024
@franky47 franky47 added this to the 🪵 Backlog milestone Nov 1, 2024
@franky47
Copy link
Member

franky47 commented Nov 20, 2024

Thanks, sorry it took so long to get into this issue.

It looks like the Remix adapter needs to follow the same optimistic search params update we use in Next.js (and really all the other adapters should follow the same pattern).

Let's assume we're typing "ab" in quick succession. The root of the issue was that, when the first request (for the first character typed, "a") is in flight, Remix's useSearchParams still returns the stale empty value, while the internal state is optimistically updated. Before it has a chance to resolve, we queue in the full "ab" value.

When the "?search=a" loader returns, the URL updates along with useSearchParams to search: "a". This causes the internal state to sync back to that temporary value, causing the flicker in the input. When the URL update finishes after queuing and the loader delay, useSearchParams resolves to search: "ab" and the UI is now eventually consistent.

The fix for this in Next was to use the useOptimistic experimental React hook. It was possible because Next app router embeds a canary version of React where useOptimistic is available, something we can't enforce for Remix users who likely use React 18.

I did some tests by following the same pattern and hacking together a useOptimisticSearchParams hook for Remix, which also adds shallow: true | false support (see my post on Bluesky).

Currently, the shallow option doesn't have an effect in Remix, everything was as if shallow was set to false, in contradiction with the docs. According to the Remix team, that is intentional: when the URL changes, loaders should re-run. In our case it's an issue as nuqs assumes local client-only updates by default, and only opts-in to server updates via shallow: false if there is an actual use-case for SSR needing the search params in loaders or Server Components in Next.

Now as to why useQueryStates works: I think that's a double bug.. 😅 It looks like the sync mechanism fails at the right moment, skipping that temporary ?search=a resolution. Another thing to look into.

@franky47
Copy link
Member

Hey, I published a couple of betas for nuqs 2.3.0, which includes better support for React Router-based frameworks (including Remix).

Using [email protected], I can no longer reproduce the issue in your provided reproduction repository.

Would you be able to give it a try in your project, and let me know if you hit any snags? I'm planning to release 2.3.0 in GA sometime next week.

@franky47
Copy link
Member

franky47 commented Jan 3, 2025

Note: [email protected] has been released with a fix for this issue.

@franky47 franky47 closed this as completed Jan 3, 2025
@gijsroge
Copy link
Author

gijsroge commented Jan 3, 2025

@franky47 thanks for all the write ups and debugging work. I kept following the updates during the holidays but had no time 😅

I just tried to update nuqs to validate if the issue is indeed resolved but I bumped into a different issue. nuqs 2.3.0 has a hard requirement on react-router >=7.0 but remix still uses 6

https://github.com/47ng/nuqs/blob/next/packages/nuqs/package.json#L132

Probably missing the ^6 ||

@franky47
Copy link
Member

franky47 commented Jan 3, 2025

Ah I see, I thought RRv7 used react-router@^7, RRv6 react-router-dom@^6 and Remix @remix-run/react, but it looks like there are a lot of cross-versions..

I'll update the version ranges to include 6 & 7 on both package names, would that work for you?

@gijsroge
Copy link
Author

gijsroge commented Jan 3, 2025

Yeah I think that should work!

@franky47
Copy link
Member

franky47 commented Jan 4, 2025

Can you try with the following preview from #844 and see if it works better please?

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

@gijsroge
Copy link
Author

gijsroge commented Jan 7, 2025

@franky47 thanks! I can now install the new version and can also confirm that the shallow option now works as expected.

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

No branches or pull requests

2 participants