Skip to content

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
roninoss:mainfrom
ShevtsovAleksey:fix/portal-host-keyed-children
Open

fix(portal): key PortalHost entries by portal name to prevent cross-portal state transfer on removal#124
ShevtsovAleksey wants to merge 1 commit into
roninoss:mainfrom
ShevtsovAleksey:fix/portal-host-keyed-children

Conversation

@ShevtsovAleksey

@ShevtsovAleksey ShevtsovAleksey commented Jul 14, 2026

Copy link
Copy Markdown

Bug

PortalHost renders its registered portals as a keyless fragment:

return <>{Array.from(portalMap.values())}</>;

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:

  • If the two portals render the same component type at the root (very common — e.g. every bottom sheet / dialog in an app renders through the same wrapper component), the later portal's tree adopts the removed portal's neighbor state: internal refs, animated values, measured layouts, open/closed state, etc. silently transfer between unrelated portals.
  • If the trees diverge in type, the later portal is torn down and remounted from scratch, losing all of its state.

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

<PortalHost />

{showA && (
  <Portal name='sheet-a'>
    <MySheet /* ... */ />
  </Portal>
)}
<Portal name='sheet-b'>
  <MySheet /* ... */ />
</Portal>

Toggle showA off while both are registered: sheet-b's subtree is reconciled onto sheet-a's old component instances and inherits their state (or remounts, if the trees diverge). With @gorhom/bottom-sheet content 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:

return (
  <>
    {Array.from(portalMap.entries()).map(([portalName, children]) => (
      <React.Fragment key={portalName}>{children}</React.Fragment>
    ))}
  </>
);

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 build and, 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

  • Bug Fixes
    • Fixed an issue where removing one portal could cause other displayed portals to be rearranged or show incorrect content.
    • Improved portal rendering stability when portals are added or removed.

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>
@vercel

vercel Bot commented Jul 14, 2026

Copy link
Copy Markdown

@ShevtsovAleksey is attempting to deploy a commit to the Ronin Technologies Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

PortalHost now gives each portal subtree a stable React key based on its portal name, preventing remaining portals from being reconciled by array position when one unmounts. A patch changeset documents the correction.

Changes

Portal host reconciliation

Layer / File(s) Summary
Key portal rendering
packages/portal/src/portal.tsx, .changeset/keyed-portal-host.md
PortalHost maps portal entries into fragments keyed by portal name, and the changeset records the patch-level fix.

Estimated code review effort: 2 (Simple) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main fix: keying PortalHost entries by portal name to prevent portal state reuse on removal.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@anton-patrushev

Copy link
Copy Markdown

@mrzachnugent would you be able to review this PR 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants