fix(core): stabilize autosave debounce and cancel on unmount in useForm #7083
Conversation
🦋 Changeset detectedLatest commit: 7316925 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for refine-doc-live-previews ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
a28689604
force-pushed
the
fix/core-useform-stable-autosave-debounce
branch
from
a10df67 to
38d322d
Compare
| const onFinishRef = React.useRef(onFinish); | ||
| React.useEffect(() => { | ||
| onFinishRef.current = onFinish; | ||
| }, [onFinish]); |
There was a problem hiding this comment.
Nice idea to prevent asyncDebounce from using a stale onFinish. But, if I'm not wrong, useEffect will be triggered on every (re-)render since onFinish is not memoized, so this could be simplified to onFinishRef.current = onFinish if you want to go with this pattern. What do you think?
There was a problem hiding this comment.
Thanks for the review!
I think it’s safer to keep the onFinishRef.current assignment inside a useEffect. This ensures the ref is only updated after commit, so it always reflects the latest committed onFinish.
If we assign it directly during render, we could risk writing an incorrect value in certain concurrent rendering scenarios, like when a render starts but later gets discarded. Wrapping it in useEffect avoids that and keeps the ref in sync with the committed tree.
There was a problem hiding this comment.
I wasn’t aware of that. Thanks for clarifying.
- Memoize the asyncDebounce instance for `onFinishAutoSave` to avoid re-creating it on each render. - Add cleanup to call `.cancel()` on unmount. - Improves autosave reliability by preserving the debounce window across renders and avoiding stray timers. Signed-off-by: Eric Chen <[email protected]>
a28689604
force-pushed
the
fix/core-useform-stable-autosave-debounce
branch
from
38d322d to
7316925
Compare
2 participants
onFinishAutoSaveto avoid re-creating it on each render..cancel()on unmount.PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
A new debounced onFinishAutoSave is created on re-renders, which resets the debounce window and can cancel pending autosaves; in rapid re-renders (e.g., typing), autosave may not fire reliably. Pending timers aren’t explicitly cancelled on unmount.
What is the new behavior?
onFinishAutoSave is memoized (one instance per debounce delay), invokes the latest onFinish via a ref, and cancels pending work on unmount. The debounce window is preserved across renders, making autosave reliable and preventing stray timers.
fixes #7082
Notes for reviewers
This is my first open-source contribution to the project, happy to adjust anything based on your feedback.