From 46879a849bc4fceca80c6eed59b6a9295aa681b0 Mon Sep 17 00:00:00 2001 From: Bao Nguyen Date: Thu, 13 Aug 2026 21:25:12 +0700 Subject: [PATCH] fix(portal): replay queued portal operations in order `PortalHost` queues portal operations that arrive before its `PortalManager` ref is attached, which is every portal that mounts in the first commit. Two bugs in that queue: - `componentDidMount` drained the queue with `pop()`, replaying the operations LIFO. Portals mounted in the same commit were therefore stacked in reverse source order. - `update()` looked up the queued operation to replace with `o.type === 'mount' || (o.type === 'update' && o.key === key)`. The first clause ignores `key`, so an update for one portal overwrote the queued mount of an unrelated one, dropping it entirely. Drain with `shift()` and key-match both branches of the lookup. --- src/components/Portal/PortalHost.tsx | 6 +- src/components/__tests__/Portal.test.tsx | 100 +++++++++++++++++++++++ 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/src/components/Portal/PortalHost.tsx b/src/components/Portal/PortalHost.tsx index ec6c98f116..c8c24afd5f 100644 --- a/src/components/Portal/PortalHost.tsx +++ b/src/components/Portal/PortalHost.tsx @@ -50,7 +50,9 @@ export default class PortalHost extends React.Component { const queue = this.queue; while (queue.length && manager) { - const action = queue.pop(); + // Replay in the order the operations were recorded, otherwise portals + // that mounted in the same commit end up stacked in reverse. + const action = queue.shift(); if (action) { switch (action.type) { case 'mount': @@ -89,7 +91,7 @@ export default class PortalHost extends React.Component { } else { const op: Operation = { type: 'mount', key, children }; const index = this.queue.findIndex( - (o) => o.type === 'mount' || (o.type === 'update' && o.key === key) + (o) => (o.type === 'mount' || o.type === 'update') && o.key === key ); if (index > -1) { diff --git a/src/components/__tests__/Portal.test.tsx b/src/components/__tests__/Portal.test.tsx index 14f07c141b..47cbf54918 100644 --- a/src/components/__tests__/Portal.test.tsx +++ b/src/components/__tests__/Portal.test.tsx @@ -1,9 +1,14 @@ +import * as React from 'react'; import { Text } from 'react-native'; import { expect, it, jest } from '@jest/globals'; import { render, screen } from '../../test-utils'; +import Dialog from '../Dialog/Dialog'; +import Modal from '../Modal'; import Portal from '../Portal/Portal'; +import PortalHost, { PortalContext } from '../Portal/PortalHost'; +import type { PortalMethods } from '../Portal/PortalHost'; jest.useRealTimers(); @@ -21,3 +26,98 @@ it('renders portal with siblings', async () => { expect(toJSON()).toMatchSnapshot(); }); + +it('renders portals in source order when mounted in the same commit', async () => { + await render( + + + first + + + second + + + third + + + ); + + const portals = screen.getAllByTestId('portal-content'); + + expect(portals).toHaveLength(3); + expect(portals[0]).toBe(screen.getByText('first')); + expect(portals[1]).toBe(screen.getByText('second')); + expect(portals[2]).toBe(screen.getByText('third')); +}); + +it('stacks components mounted in the same commit in source order', async () => { + await render( + + + {}}> + modal + + + + {}}> + dialog + + + + ); + + // The last portal in source order is painted on top, so `dialog` has to come + // after `modal` in the rendered tree. + const layers = await screen.findAllByTestId('layer'); + + expect(layers).toHaveLength(2); + expect(layers[0]).toBe(screen.getByText('modal')); + expect(layers[1]).toBe(screen.getByText('dialog')); +}); + +it('keeps queued mounts of other portals when one of them is updated', async () => { + // Mirrors `PortalConsumer`: mounts from `componentDidMount`, then updates its + // own key. Both calls land before `PortalHost` attaches its manager, so they + // go through the queue. + class QueuedConsumer extends React.Component<{ + manager: PortalMethods; + label: string; + update?: boolean; + }> { + componentDidMount() { + const key = this.props.manager.mount( + {this.props.label} + ); + + if (this.props.update) { + this.props.manager.update( + key, + {`${this.props.label} (updated)`} + ); + } + } + + render() { + return null; + } + } + + await render( + + + {(manager) => ( + <> + + + + )} + + + ); + + // Deliberately order-independent: this asserts that no portal is lost, which + // is a separate concern from the order the queue is replayed in. + expect(screen.getAllByTestId('queued')).toHaveLength(2); + expect(screen.getByText('first')).toBeTruthy(); + expect(screen.getByText('second (updated)')).toBeTruthy(); +});