fix(react): stabilize user/users/modules props to prevent re-init flicker (SD-2635)#2876
Conversation
β¦cker (SD-2635) Consumers passing inline object literals (the idiomatic React pattern) caused a full SuperDoc destroy + rebuild on every parent re-render because the main useEffect compared these props by reference identity. When a consumer stored the SuperDoc instance in state from onReady, the resulting re-render supplied fresh references and triggered another cycle β observed as 4 full destroy/re-init cycles in ~100ms with visible display:none/block flicker. Wrap user, users, and modules in a new useStableValue hook that returns a reference-stable version only changing identity when the structural content actually changes (via JSON.stringify compare, run only on reference-change). Semantics are strictly a superset of the prior behavior β value changes still rebuild; reference-only changes no longer do.
There was a problem hiding this comment.
π‘ Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ed6375ad3
βΉοΈ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with π.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (a === b) return true; | ||
| if (a == null || b == null) return a === b; | ||
| try { | ||
| return JSON.stringify(a) === JSON.stringify(b); |
There was a problem hiding this comment.
Detect function-valued config changes
useStableValue currently treats two objects as equal when their only differences are function properties, because JSON.stringify drops functions. SuperDocEditor now applies this hook to modules, but modules supports function-valued options (for example modules.links.popoverResolver), so updating that callback can be silently ignored and the editor will keep using stale behavior until a different rebuild trigger occurs. This is a regression from the previous reference-based dependency behavior for module callbacks.
Useful? React with πΒ / π.
Consumers who pass
user,users, ormodulesas inline object literals β the idiomatic React pattern β trigger a full SuperDoc destroy + rebuild on every parent re-render. When a consumer stores the instance in state fromonReady, the re-render that follows produces a new object reference and the cycle repeats./instrumentcaptured four complete destroy/re-init cycles in ~100ms with visibledisplay:none/blockflicker and repeated[Vue warn]: Cannot unmount an app that is not mounted.Root cause is in the main
useEffectdeps onSuperDocEditor.tsx:229: React compares those props by reference. The original intent (clear from the existing comment) was to rebuild on value change β the mechanism was just too strict. Wrappinguser,users, andmodulesin a newuseStableValuehook makes identity track content instead of reference. Semantics are a strict superset of before: value changes still rebuild; reference-only changes no longer do.useStableValue(stable across identical content, rebuilds on value change, null/undefined, arrays, circular-ref fallback)display: none, 4 re-init cycles. After β document renders, single init, no warnings.Rejected: deep-equal dep (adds surface for a one-hook problem); removing
user/users/modulesfrom deps entirely (breaks consumers who rely on rebuild on value change).Review: check the JSON.stringify fallback on circular refs in
utils.ts:structurallyEqual. Ignore theanywarnings on the pre-existinguseStableIdpolyfill.Closes SD-2635.