fix: Regression in mounting components #461
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
franky47
force-pushed
the
fix/359-regression
branch
from
2fc5adc
to
b4a9063
Compare
Since applying the search params to the URL is deferred, it causes the 359 regression: on mount, the component with a useQueryState identical to the one that triggered its mounting would have a correct internal state, but that would get overriden by a stale (not yet updated) iSP. Trying to remove the whole sync logic to see how previous versions may break, and adding 14.0.5-canary.54 which stabilises WHS for future-proofing until GA.
franky47
force-pushed
the
fix/359-regression
branch
from
b4a9063
to
c1f8843
Compare
We'll release a patch fix for the 1.x release line and this is going straight to the bin in v2, it will only support 14.0.4 onwards.
|
🎉 This PR is included in version 1.15.3 🎉 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Regression introduced by #457 was detected in the repro-359 test case, where components with a
useQueryStatefailed to capture the correct value on mount, and trailed behind.Root of the issue
The
useEffectintroduced in #394 to sync internal state against a reactiveuseSearchParamshook in order to support WHS in 14.0.3 had a flaw. Actually, 14.0.3 had a flaw too, but we'll come to that in a bit.The test in #359 exposes differences between the internal state of the
useQueryStatehook and the URL state (as reflected in a reactiveuseSearchParams): when a component mounts because of a query state change, and that component also listens for the same query key, its internal state will be fine on mount (due to the state initialisation function taking into account the URL update queue), but the initialuseSearchParamswill still reflect the stale, not-yet-updated URL. Attempting to sync on that on mount causes the value to trail behind.Now onto the issue in
[email protected]: implementation of the WHS had a bug that didn't call patched history methods made in third party modules likenuqs(and other analytics packages). This caused a known issue to be published, with the resolution of "enable WHS". This would only help make theuseSearchParamshook reactive, and we can sync to it, but only in 14.0.3, leading to the ugly hack of version detection that will be removed entirely in v2 (along with support for 14.0.3).So is 14.0.3, with WHS, subject to 359? Turns out, no. For some reason the tests pass fine there. No idea why, as it theoretically should fail, but since we're going to drop it anyway.. 🤷