fix(portal): key PortalHost entries by portal name to prevent cross-portal state transfer on removal#124
Open
ShevtsovAleksey wants to merge 1 commit into
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 |
anton-patrushev
approved these changes
Jul 14, 2026
|
@mrzachnugent would you be able to review this PR 🙏 |
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.
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