fix(portal): key PortalHost entries by portal name to prevent cross-portal state transfer on removal#124
Conversation
PortalHost rendered its portals as a keyless fragment, so React reconciled them by array position. Removing one portal slid every later portal onto its predecessor's component instances: same-type portals silently inherited the predecessor's internal state, and diverging ones were torn down and remounted. Keying each portal's subtree by its unique portal name makes removals local to the removed portal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@ShevtsovAleksey is attempting to deploy a commit to the Ronin Technologies Team on Vercel. A member of the Team first needs to authorize it. |
Walkthrough
ChangesPortal host reconciliation
Estimated code review effort: 2 (Simple) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@mrzachnugent would you be able to review this PR 🙏 |
|
@mrzachnugent hey there! friendly bump on this one - tested this in a real app - the fix is doing the job! |
Bug
PortalHostrenders its registered portals as a keyless fragment:Without keys, React reconciles the children by array position. When a portal is removed, every portal registered after it slides one position down and is reconciled against its predecessor's element:
This is invisible while portal membership is static, but bites as soon as portals mount/unmount dynamically (e.g. a sheet that mounts on open and unmounts after close): closing one sheet scrambles or remounts every sheet registered after it in the same host.
Repro sketch
Toggle
showAoff while both are registered:sheet-b's subtree is reconciled ontosheet-a's old component instances and inherits their state (or remounts, if the trees diverge). With@gorhom/bottom-sheetcontent this shows up as sheets losing their footer, inheriting another sheet's snap position, or disappearing when an unrelated sheet closes.Fix
Key each portal's subtree by its portal name, which is already unique per host:
Removals become local to the removed portal; the other portals keep their component instances. No public API change. A changeset (patch bump for
@rn-primitives/portal) is included.Verified with
pnpm --filter @rn-primitives/portal buildand, in a production app, with a regression test that mounts two same-type portal subtrees, removes the first, and asserts the second neither remounts nor swaps component instances — it fails on the keyless host and passes with this change.Summary by CodeRabbit