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: Don't reset shallow URL updates on prefetch #58417

Conversation

anonkey
Copy link

@anonkey anonkey commented Nov 14, 2023

Original PR #58297 from franky47

Description
Between 14.0.2-canary.6 and 14.0.2-canary.7, a change was introduced in #56497 that turned the Redux store state into a Promise, rather than a synchronous state update.

This caused the sync function -- used to send state updates to the Redux Devtools -- to be recreated on every dispatch, which in turn, by referential instability, caused the HistoryUpdater component to re-render and trigger a history.replaceState with no particular change, but with the internal canonicalUrl.

When an app does a soft/shallow navigation by calling history methods directly (currently the only way to do shallow search params updates in the app router), these changes would have been overwritten by any prefetch (eg: hovering or mounting a Link), which is usually a no-op for the navigation state.

This PR removes the sync function for the Redux devtools, as it cannot function as-is: actual state updates need to be awaited and forwarded somewhere else in the router if this behaviour is to be maintained, and should be decoupled from history calls to prevent side-effects.

Context
I maintain next-usequerystate, which is used in the Vercel dashboard, and which is impacted by this change (see
47ng/nuqs#388).

History
@timneutkens introduced the sync function and the whole Redux devtools reducer in #39866, with the note:

a new hook useReducerWithReduxDevtools has been added, we'll probably want to put this behind a compile-time flag when the new router is marked stable but until then it's useful to have it enabled by default (only
when you have Redux Devtools installed ofcourse).

If a different direction is needed to keep sending RENDER_SYNC actions to Redux devtools, I'll be happy to rework this PR to move the sync function into the action queue.

Changes
Added e2e test. Requires a start mode as prefetch links are disabled in development. Test was verified to fail from next@>=12.0.2-canary.7 without the fix.

@franky47
Copy link
Contributor

Merge conflicts fixed in base PR #58297, you can test there @anonkey. Thanks!

@ztanner
Copy link
Member

ztanner commented Nov 14, 2023

Closing as #58297 was merged. Thanks!

@ztanner ztanner closed this Nov 14, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants