[popups] Don't let a late trigger claim a programmatically-opened popup

A trigger that mounts while a popup is already open (via a handle's
openWithPayload/open(null)) would claim ownership and overwrite the
programmatic payload. Track openedWithoutTrigger in the popup store and skip
the implicit, registration-time, and single-trigger ARIA (aria-controls) claim
paths for such opens; reset the flag on close so a later non-imperative open
(controlled/defaultOpen) isn't affected. DialogHandle.open records trigger
intent so handle.open(id) before mount still associates. Adds regression tests
for payload preservation and the absent ARIA association. (B5)

Author: atomiks

packages/react/src/dialog/root/DialogRoot.detached-triggers.test.tsx

@@ -787,5 +787,78 @@ describe('<Dialog.Root />', () => {
         expect(screen.queryByRole('dialog')).toBe(null);
       });
     });
+
+    it('does not overwrite a programmatic payload when a trigger mounts while open', async () => {
+      const dialog = Dialog.createHandle<number>();
+
+      function App({ showTrigger }: { showTrigger: boolean }) {
+        return (
+          <div>
+            {showTrigger && (
+              <Dialog.Trigger handle={dialog} id="late-trigger" payload={42}>
+                Late trigger
+              </Dialog.Trigger>
+            )}
+            <Dialog.Root handle={dialog}>
+              {({ payload }: { payload: number | undefined }) => (
+                <Dialog.Portal>
+                  <Dialog.Popup data-testid="content">{payload}</Dialog.Popup>
+                </Dialog.Portal>
+              )}
+            </Dialog.Root>
+          </div>
+        );
+      }
+
+      const { setProps } = await render(<App showTrigger={false} />);
+
+      await act(() => dialog.openWithPayload(8));
+      await waitFor(() => {
+        expect(screen.queryByRole('dialog')).not.toBe(null);
+      });
+      expect(screen.getByTestId('content').textContent).toBe('8');
+
+      // Mounting a trigger after the dialog was opened programmatically must not overwrite
+      // the payload set via `openWithPayload`.
+      await setProps({ showTrigger: true });
+
+      expect(screen.getByTestId('content').textContent).toBe('8');
+    });
+
+    it('does not associate a late single trigger with a programmatically-opened dialog', async () => {
+      const dialog = Dialog.createHandle<number>();
+
+      function App({ showTrigger }: { showTrigger: boolean }) {
+        return (
+          <div>
+            {showTrigger && (
+              <Dialog.Trigger data-testid="late-trigger" handle={dialog} id="late-trigger">
+                Late trigger
+              </Dialog.Trigger>
+            )}
+            <Dialog.Root handle={dialog} modal={false}>
+              <Dialog.Portal>
+                <Dialog.Popup data-testid="content">Content</Dialog.Popup>
+              </Dialog.Portal>
+            </Dialog.Root>
+          </div>
+        );
+      }
+
+      const { setProps } = await render(<App showTrigger={false} />);
+
+      await act(() => dialog.openWithPayload(1));
+      await waitFor(() => {
+        expect(screen.queryByRole('dialog')).not.toBe(null);
+      });
+
+      await setProps({ showTrigger: true });
+
+      const trigger = screen.getByTestId('late-trigger');
+      // The dialog was opened without a trigger, so a late single trigger must not claim an
+      // ARIA relationship (aria-controls / aria-expanded) to a popup it didn't open.
+      expect(trigger).not.toHaveAttribute('aria-controls');
+      expect(trigger).toHaveAttribute('aria-expanded', 'false');
+    });
   });
 });

packages/react/src/dialog/store/DialogHandle.ts

@@ -41,6 +41,12 @@ export class DialogHandle<Payload> {
       true,
       createChangeEventDetails(REASONS.imperativeAction, undefined, triggerElement),
     );
+
+    if (triggerId && !triggerElement) {
+      // The intended trigger hasn't mounted yet. Record the association so it is claimed
+      // (with its payload) once it registers, instead of being treated as a trigger-less open.
+      this.store.update({ activeTriggerId: triggerId, openedWithoutTrigger: false });
+    }
   }
 
   /**

packages/react/src/utils/popups/popupStoreUtils.ts

@@ -136,6 +136,12 @@ export function setPopupOpenState(
 
   const triggerId = trigger?.id ?? null;
 
+  if (open) {
+    // Track whether this open was initiated without a trigger (e.g. `openWithPayload`), so a
+    // later-mounting trigger doesn't claim ownership and overwrite programmatically set state.
+    state.openedWithoutTrigger = trigger == null;
+  }
+
   // If a popup is closing, the `trigger` may be undefined.
   // We want to keep the previous value so that exit animations are played and focus is returned correctly.
   if (triggerId || open) {
@@ -209,10 +215,11 @@ export function useTriggerDataForwarding<State extends PopupStoreState<unknown>>
       return;
     }
 
-    if (activeTriggerId == null && open) {
+    if (activeTriggerId == null && open && !store.state.openedWithoutTrigger) {
       // If a popup is already open, a detached trigger can mount before any active trigger
       // has been established. Claim the first registered trigger so trigger-owned focus
-      // management and ARIA relationships work.
+      // management and ARIA relationships work. Popups opened imperatively without a trigger
+      // (`openedWithoutTrigger`) are excluded so a late trigger can't overwrite their payload.
       store.update({
         activeTriggerId: triggerId,
         activeTriggerElement: element,
@@ -270,6 +277,12 @@ export function useImplicitActiveTrigger<State extends PopupStoreState<unknown>>
       if (store.state.triggerCount !== 0) {
         store.set('triggerCount', 0);
       }
+      // Reset the triggerless-open flag once closed so a later open through a path that
+      // doesn't run `setPopupOpenState` (e.g. controlled `open`/`defaultOpen`) isn't
+      // suppressed by a stale value.
+      if (store.state.openedWithoutTrigger) {
+        store.set('openedWithoutTrigger', false);
+      }
       return;
     }
 
@@ -292,7 +305,12 @@ export function useImplicitActiveTrigger<State extends PopupStoreState<unknown>>
       }
     }
 
-    if (!lostActiveTriggerId && !activeTriggerId && triggerCount === 1) {
+    if (
+      !lostActiveTriggerId &&
+      !activeTriggerId &&
+      triggerCount === 1 &&
+      !store.state.openedWithoutTrigger
+    ) {
       const iteratorResult = store.context.triggerElements.entries().next();
       if (!iteratorResult.done) {
         const [implicitTriggerId, implicitTriggerElement] = iteratorResult.value;

packages/react/src/utils/popups/store.ts

@@ -54,6 +54,12 @@ export type PopupStoreState<Payload> = {
    * The currently active trigger DOM element.
    */
   activeTriggerElement: Element | null;
+  /**
+   * Whether the popup was opened imperatively without a trigger (e.g. via a handle's
+   * `openWithPayload`/`open(null)`). When set, a later-mounting trigger must not claim ownership
+   * and overwrite programmatically set state such as `payload`.
+   */
+  openedWithoutTrigger: boolean;
   /**
    * ID of the trigger (external prop).
    */
@@ -94,6 +100,7 @@ export function createInitialPopupStoreState<Payload>(): PopupStoreState<Payload
     payload: undefined,
     activeTriggerId: null,
     activeTriggerElement: null,
+    openedWithoutTrigger: false,
     triggerIdProp: undefined,
     popupElement: null,
     positionerElement: null,
@@ -168,7 +175,9 @@ function triggerOwnsOpenPopupOrIsOnlyTrigger(state: S, triggerId: string | undef
     triggerId !== undefined &&
     openSelector(state) &&
     activeTriggerIdSelector(state) == null &&
-    state.triggerCount === 1
+    state.triggerCount === 1 &&
+    // A popup opened without a trigger must not be associated with a lone late trigger.
+    !state.openedWithoutTrigger
   );
 }