fix(LineSidebar, OptionWheel): clear rafRef on cleanup so the loop restarts under StrictMode (#1014) - #1019
Open
noron12234 wants to merge 1 commit into
Open
Conversation
…n restart The unmount cleanup cancels the pending frame but leaves the id in rafRef.current. React StrictMode runs cleanup and then re-runs the effect on the same component instance, so on the second mount startLoop() sees a non-null rafRef.current, takes the 'a loop is already running' early return, and the loop is never scheduled again. Result under StrictMode: OptionWheel never applies transforms and LineSidebar never updates --effect. Closes DavidHDev#1014. Applied to all four variants of both components.
This file contains hidden or 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
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.
Closes #1014.
Root cause
The unmount cleanup cancels the frame but never clears the id:
React StrictMode runs cleanup and then re-runs the effect on the same instance, so the second mount hits:
and the loop is never scheduled again.
runFramedoes clear the ref correctly when the animation settles, so the ref is only ever left stale by the cleanup path — which is exactly the StrictMode remount.That also explains why this is invisible in this repo: the docs site's
src/main.jsxdoes not wrap the app inStrictMode, while the Vite/Next scaffolds consumers start from do.Reproduction
Stock
npm create vite@latest -- --template react-tsapp (which renders inside<StrictMode>), officialTS-TWsources, hovering the middle of the list:--effectset onmain, StrictMode onmain, StrictMode off (control)Values with StrictMode on after the fix are identical to the StrictMode-off control (
0.7183, 0.9231, 1.0000, 0.9054, …), so nothing else changed.Same A/B for OptionWheel, counting elements that received a wheel transform:
main, StrictMode ontranslate(-14.02px, calc(-50% - 133.42px)) rotate(-12deg), …)No console errors in any run.
Why this fix rather than the one in the issue
#1014 suggests making
startLoop()alwayscancelAnimationFrameand restart. That works, but it also throws away the "one loop at a time" invariant and resetslastRefon every call, so astartLoop()during an in-flight animation would restart the frame-timing baseline. The ref is only ever left stale by the cleanup, so clearing it there fixes the cause and leaves the running-loop behaviour untouched. Happy to switch to the other shape if you prefer it."Possibly other components using the same animation loop pattern"
Checked. The defect needs all three of: the rAF id stored in a
useRef(survives a StrictMode remount), an early-return guard on that ref, and a cancel that does not reset it. Scanning every.jsx/.tsxundersrc/for that combination returns exactly these two components and nothing else — the other ~60 rAF components either keep the id in an effect-scoped local (recreated on remount) or have no early-return guard.Scope
All four variants of both components (8 files), per the contributing guide. Only the cleanup block changed.
Note: these 8 files already fail
prettier --checkonmain, and they still do — I did not reformat them, so the diff stays limited to the fix.