fix: reset the testing adapter's URL update queue once per mount - #1518
fix: reset the testing adapter's URL update queue once per mount#1518inf1nite-lo0p wants to merge 2 commits into
Conversation
|
@inf1nite-lo0p is attempting to deploy a commit to the 47ng Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
franky47
added
the
adapters/testing
franky47
added
the
deploy:preview
commit: |
|
|
||
| const waitForSettle = () => | ||
| new Promise<void>(resolve => { | ||
| setTimeout(resolve, 100) |
| let value: string | null = null | ||
| let setValue: (update: string | null) => Promise<unknown> = () => |
There was a problem hiding this comment.
nitpick: I'd rather avoid closing on component internals, a more isolated way would be to render a test button to call the setter sequence and act on it.
franky47
added
the
pr-waiting-for-feedback
2 participants
Fixes #1517.
NuqsTestingAdaptercallsresetQueues()in the component body, guarded only by theresetUrlUpdateQueueOnMountprop, so despite the prop's name it runs on every render. WithhasMemory, every flushed URL update re-renders the adapter (updateUrlcallssetSearchParams). Combined, an update enqueued while that re-render is pending gets aborted by the adapter's own render: it never reaches the URL, and the abort's re-sync reverts every subscribed hook to the URL's stale value.Following the contributing guide's suggestion, the first commit is the failing test and the second is the fix:
test:a browser test that writes a value, then chains a second write on the first write's promise. The promise resolves at the end of the flush, so the second write is enqueued in a microtask before the adapter's pending re-render commits. Onnextthis fails withonUrlUpdatecalled once instead of twice: the second write is aborted by the adapter's render body and the state reverts.fix:a ref guard so the reset runs once per mounted instance. The guard stays in the component body on purpose: moving it to an effect would run it after children's mount-time writes and abort those instead.Verified locally: the new test fails on
nextwithout the fix and passes with it;pnpm --filter nuqs test:unit262/262 andtest:typespass; Prettier clean.Where this was found, for context: a dashboard whose sheet stack lives in a
?sheets=param. A close dispatched right after an open's flush was silently dropped about 1 run in 15 locally and far more often on loaded CI runners, leaving the sheet permanently open in tests. Deterministic standalone reproduction with traces: https://github.com/inf1nite-lo0p/nuqs-1517-repro