Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where useHandler's doDependenciesDiffer flag was incorrectly set to false after the first hot reload (fast refresh), causing downstream consumers like React Native Gesture Handler (RNGH) to not rebuild their handlers. It introduces a fast refresh detection mechanism and fixes a race condition in the useEffect cleanup.
Changes:
- Added a
useHasFastRefreshedutility hook that detects fast refresh by comparinguseMemo(reset by React on refresh) withuseRef(preserved on refresh) - Modified
useHandlerto reset its internal state and forcedoDependenciesDiffer = truewhen a fast refresh is detected - Fixed a race condition where the
useEffectcleanup could null out a freshly re-initializedinitRefby guarding the cleanup with a reference identity check
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/react-native-reanimated/src/hook/utils.ts |
Added useHasFastRefreshed hook and its useMemo/useRef imports |
packages/react-native-reanimated/src/hook/useHandler.ts |
Integrated fast refresh detection, reset initRef on refresh, guarded cleanup against race conditions, and included isFastRefresh in doDependenciesDiffer computation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I'm blocking this PR due to #9081 |
8486b93 to
e7de456
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const signal = useMemo(() => ({}), []); | ||
| const prev = useRef(signal); | ||
| if (prev.current === signal) { | ||
| return false; | ||
| } | ||
| prev.current = signal; | ||
| return true; |
| // On fast refresh we want the hook to behave as a first render rather | ||
| // than a re-render, so `doDependenciesDiffer` is reported as true and | ||
| // consumers can rebuild native bindings tied to pre-refresh JS identities. | ||
| // See https://github.com/software-mansion/react-native-reanimated/pull/9075. | ||
| const isFastRefresh = useIsFastRefresh(); | ||
| if (isFastRefresh) { | ||
| stateRef.current = null; | ||
| } |
Summary
The
doDependenciesDifferflag is set to false after the first hot reload (it is true after all other reloads). This PR adds a check whether we have a hot reload, and setdoDependenciesDifferto true in that case.Moreover, I noticed that the cleanup in the
useEffectdoesn't check whether the ref it cleans is the one it started with. Due to some race conditions after a hot reload the cleanup runs just after the mount, cleaning the newly attached ref. I added a check in cleanup.The motivation for the fix is that RNGH relies on the
doDependenciesDifferflag to rebuild its handlers. See PR with a workaround in RNGH: software-mansion/react-native-gesture-handler#3997Test plan
Tested on the following example:
Details
Before the fix, on first reload the
doDependenciesDifferflag was set to false after first hot reload, this caused issues in RNGH (see software-mansion/react-native-gesture-handler#3997).After the fix it is true on every hot reload.