RAC Menu not able to click menu items when using UNSAFE_PortalProvider in a web component

[johnpangalos]

Provide a general summary of the issue here

When using React Aria Components within a Shadow DOM setup that includes an UNSAFE_PortalProvider, menu items in popovers close immediately upon clicking instead of executing their onAction handler.

🤔 Expected Behavior?

Clicking a menu item should trigger the onAction handler with the item's ID and show an alert.

😯 Current Behavior

Clicking a menu item immediately closes the popover without executing the action handler.

💁 Possible Solution

We applied these patches which got things somewhat working, but it needed more work.

diff --git a/dist/FocusScope.mjs b/dist/FocusScope.mjs
index 60aeeb079b6e702a4484e23a1b331856567bc39d..598c1d64e0053565683f8e590d31abff76839942 100644
--- a/dist/FocusScope.mjs
+++ b/dist/FocusScope.mjs
@@ -1,4 +1,4 @@
-import {useLayoutEffect as $cgawC$useLayoutEffect, getActiveElement as $cgawC$getActiveElement, getOwnerDocument as $cgawC$getOwnerDocument, getEventTarget as $cgawC$getEventTarget, isAndroid as $cgawC$isAndroid, isChrome as $cgawC$isChrome, isTabbable as $cgawC$isTabbable, isFocusable as $cgawC$isFocusable, createShadowTreeWalker as $cgawC$createShadowTreeWalker} from "@react-aria/utils";
+import {useLayoutEffect as $cgawC$useLayoutEffect, getActiveElement as $cgawC$getActiveElement, getOwnerDocument as $cgawC$getOwnerDocument, getEventTarget as $cgawC$getEventTarget, isAndroid as $cgawC$isAndroid, isChrome as $cgawC$isChrome, isTabbable as $cgawC$isTabbable, isFocusable as $cgawC$isFocusable, createShadowTreeWalker as $cgawC$createShadowTreeWalker, nodeContains} from "@react-aria/utils";
 import {getInteractionModality as $cgawC$getInteractionModality, focusSafely as $cgawC$focusSafely} from "@react-aria/interactions";
 import $cgawC$react, {useRef as $cgawC$useRef, useContext as $cgawC$useContext, useMemo as $cgawC$useMemo, useEffect as $cgawC$useEffect} from "react";
 
@@ -324,7 +324,7 @@ function $9bf71ea28793e738$var$isElementInAnyScope(element) {
 function $9bf71ea28793e738$var$isElementInScope(element, scope) {
     if (!element) return false;
     if (!scope) return false;
-    return scope.some((node)=>node.contains(element));
+    return scope.some((node)=>nodeContains(node, element));
 }
 function $9bf71ea28793e738$var$isElementInChildScope(element, scope = null) {
     // If the element is within a top layer element (e.g. toasts), always allow moving focus there.
@@ -573,7 +573,7 @@ function $9bf71ea28793e738$export$2d6ec8fc375ceafa(root, opts, scope) {
         acceptNode (node) {
             var _opts_from;
             // Skip nodes inside the starting node.
-            if (opts === null || opts === void 0 ? void 0 : (_opts_from = opts.from) === null || _opts_from === void 0 ? void 0 : _opts_from.contains(node)) return NodeFilter.FILTER_REJECT;
+            if (opts === null || opts === void 0 ? void 0 : (_opts_from = opts.from) === null || _opts_from === void 0 ? void 0 : nodeContains(_opts_from, node)) return NodeFilter.FILTER_REJECT;
             if ((opts === null || opts === void 0 ? void 0 : opts.tabbable) && node.tagName === 'INPUT' && node.getAttribute('type') === 'radio') {
                 // If the radio is in a form, we can get all the other radios by name
                 if (!$9bf71ea28793e738$var$isTabbableRadio(node)) return NodeFilter.FILTER_REJECT;
@@ -598,7 +598,7 @@ function $9bf71ea28793e738$export$c5251b9e124bf29(ref, defaultOptions = {}) {
                 tabbable: tabbable,
                 accept: accept
             });
-            if (root.contains(node)) walker.currentNode = node;
+            if (nodeContains(root, node)) walker.currentNode = node;
             let nextNode = walker.nextNode();
             if (!nextNode && wrap) {
                 walker.currentNode = root;
@@ -616,7 +616,7 @@ function $9bf71ea28793e738$export$c5251b9e124bf29(ref, defaultOptions = {}) {
                 tabbable: tabbable,
                 accept: accept
             });
-            if (root.contains(node)) walker.currentNode = node;
+            if (nodeContains(root, node)) walker.currentNode = node;
             else {
                 let next = $9bf71ea28793e738$var$last(walker);
                 if (next) $9bf71ea28793e738$var$focusElement(next, true);

diff --git a/dist/useFocusWithin.mjs b/dist/useFocusWithin.mjs
index 35d67252457199428e809e42e970559bbabfd993..339e80a8425e60c91bf7a3a695eb3fb48de9b1b6 100644
--- a/dist/useFocusWithin.mjs
+++ b/dist/useFocusWithin.mjs
@@ -27,11 +27,11 @@ function $9ab94262bd0047c7$export$420e68273165f4ec(props) {
     let { addGlobalListener: addGlobalListener, removeAllGlobalListeners: removeAllGlobalListeners } = (0, $3b9Q0$useGlobalListeners)();
     let onBlur = (0, $3b9Q0$useCallback)((e)=>{
         // Ignore events bubbling through portals.
-        if (!e.currentTarget.contains(e.target)) return;
+        if (!$3b9Q0$nodeContains(e.currentTarget.contains, e.target)) return;
         // We don't want to trigger onBlurWithin and then immediately onFocusWithin again
         // when moving focus inside the element. Only trigger if the currentTarget doesn't
         // include the relatedTarget (where focus is moving).
-        if (state.current.isFocusWithin && !e.currentTarget.contains(e.relatedTarget)) {
+        if (state.current.isFocusWithin && !$3b9Q0$nodeContains(e.currentTarget, e.relatedTarget)) {
             state.current.isFocusWithin = false;
             removeAllGlobalListeners();
             if (onBlurWithin) onBlurWithin(e);
@@ -46,7 +46,7 @@ function $9ab94262bd0047c7$export$420e68273165f4ec(props) {
     let onSyntheticFocus = (0, $8a9cb279dc87e130$export$715c682d09d639cc)(onBlur);
     let onFocus = (0, $3b9Q0$useCallback)((e)=>{
         // Ignore events bubbling through portals.
-        if (!e.currentTarget.contains(e.target)) return;
+        if (!$3b9Q0$nodeContains(e.currentTarget, e.target)) return;
         // Double check that document.activeElement actually matches e.target in case a previously chained
         // focus handler already moved focus somewhere else.
         const ownerDocument = (0, $3b9Q0$getOwnerDocument)(e.target);
diff --git a/dist/useInteractOutside.mjs b/dist/useInteractOutside.mjs
index e2d5c3879a725155ef8469396632baec50b7f851..2ccb24f6216a875126accdd4807a11c8efc0c114 100644
--- a/dist/useInteractOutside.mjs
+++ b/dist/useInteractOutside.mjs
@@ -1,4 +1,4 @@
-import {useEffectEvent as $ispOf$useEffectEvent, getOwnerDocument as $ispOf$getOwnerDocument} from "@react-aria/utils";
+import {useEffectEvent as $ispOf$useEffectEvent, getOwnerDocument as $ispOf$getOwnerDocument, nodeContains} from "@react-aria/utils";
 import {useRef as $ispOf$useRef, useEffect as $ispOf$useEffect} from "react";
 
 /*
@@ -86,7 +86,7 @@ function $e0b6e0b68ec7f50f$var$isValidEvent(event, ref) {
     if (event.target) {
         // if the event target is no longer in the document, ignore
         const ownerDocument = event.target.ownerDocument;
-        if (!ownerDocument || !ownerDocument.documentElement.contains(event.target)) return false;
+        if (!ownerDocument || !nodeContains(ownerDocument.documentElement, event.target)) return false;
         // If the target is within a top layer element (e.g. toasts), ignore.
         if (event.target.closest('[data-react-aria-top-layer]')) return false;
     }

diff --git a/dist/ariaHideOutside.mjs b/dist/ariaHideOutside.mjs
index e74461e87c0279fe1933e52a4ebff1e7f21bf516..dc228dd1ac3e8423a1cb9ce41ef14a746e0cae67 100644
--- a/dist/ariaHideOutside.mjs
+++ b/dist/ariaHideOutside.mjs
@@ -1,4 +1,4 @@
-import {getOwnerWindow as $fF94N$getOwnerWindow} from "@react-aria/utils";
+import {getOwnerWindow as $fF94N$getOwnerWindow, getOwnerDocument, createShadowTreeWalker, nodeContains} from "@react-aria/utils";
 
 /*
  * Copyright 2020 Adobe. All rights reserved.
@@ -45,13 +45,19 @@ function $5e3802645cc19319$export$1c3ebcada18427bf(targets, options) {
             if (hiddenNodes.has(node) || visibleNodes.has(node) || node.parentElement && hiddenNodes.has(node.parentElement) && node.parentElement.getAttribute('role') !== 'row') return NodeFilter.FILTER_REJECT;
             // Skip this node but continue to children if one of the targets is inside the node.
             for (let target of visibleNodes){
-                if (node.contains(target)) return NodeFilter.FILTER_SKIP;
+                if (nodeContains(node, target)) return NodeFilter.FILTER_SKIP;
             }
             return NodeFilter.FILTER_ACCEPT;
         };
-        let walker = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, {
-            acceptNode: acceptNode
-        });
+        let rootElement = root?.nodeType === Node.ELEMENT_NODE ? (root) : null;
+        let doc = getOwnerDocument(rootElement);
+        let walker = createShadowTreeWalker(
+          doc,
+          root || doc,
+          NodeFilter.SHOW_ELEMENT,
+          { acceptNode }
+        );
+        
         // TreeWalker does not include the root.
         let acceptRoot = acceptNode(root);
         if (acceptRoot === NodeFilter.FILTER_ACCEPT) hide(root);
@@ -85,7 +91,7 @@ function $5e3802645cc19319$export$1c3ebcada18427bf(targets, options) {
             if (![
                 ...visibleNodes,
                 ...hiddenNodes
-            ].some((node)=>node.contains(change.target))) for (let node of change.addedNodes){
+            ].some((node)=>nodeContains(node,change.target))) for (let node of change.addedNodes){
                 if ((node instanceof HTMLElement || node instanceof SVGElement) && (node.dataset.liveAnnouncer === 'true' || node.dataset.reactAriaTopLayer === 'true')) visibleNodes.add(node);
                 else if (node instanceof Element) walk(node);
             }

diff --git a/dist/DOMFunctions.mjs b/dist/DOMFunctions.mjs
index e3d616b7c31cb1c61b1b377e04b22cfaea67ca9a..276efe650739c4d1a75fe08463e8615be06eae82 100644
--- a/dist/DOMFunctions.mjs
+++ b/dist/DOMFunctions.mjs
@@ -26,7 +26,7 @@ const $d4ee10de306f2510$export$cd4e5573fbe2b576 = (doc = document)=>{
     return activeElement;
 };
 function $d4ee10de306f2510$export$e58f029f0fbfdb29(event) {
-    if ((0, $lcSu5$shadowDOM)() && event.target.shadowRoot) {
+    if ((0, $lcSu5$shadowDOM)() && event.target?.shadowRoot) {
         if (event.composedPath) return event.composedPath()[0];
     }
     return event.target;

diff --git a/dist/Popover.mjs b/dist/Popover.mjs
index be1f4ad9ef5fcf9869d4debcb48723a89327499d..366a154cbea7913a54829771dc63a50bf94902ff 100644
--- a/dist/Popover.mjs
+++ b/dist/Popover.mjs
@@ -2,7 +2,7 @@ import {useContextProps as $64fa3d84918910a7$export$29f1550f4b0d4415, useRenderP
 import {OverlayArrowContext as $44f671af83e7d9e0$export$2de4954e8ae13b9f} from "./OverlayArrow.mjs";
 import {OverlayTriggerStateContext as $de32f1b87079253c$export$d2f961adcb0afbe} from "./Dialog.mjs";
 import {useLocale as $ehFet$useLocale, usePopover as $ehFet$usePopover, DismissButton as $ehFet$DismissButton, Overlay as $ehFet$Overlay} from "react-aria";
-import {useExitAnimation as $ehFet$useExitAnimation, useLayoutEffect as $ehFet$useLayoutEffect, useEnterAnimation as $ehFet$useEnterAnimation, mergeProps as $ehFet$mergeProps, filterDOMProps as $ehFet$filterDOMProps} from "@react-aria/utils";
+import {useExitAnimation as $ehFet$useExitAnimation, useLayoutEffect as $ehFet$useLayoutEffect, useEnterAnimation as $ehFet$useEnterAnimation, mergeProps as $ehFet$mergeProps, filterDOMProps as $ehFet$filterDOMProps, nodeContains} from "@react-aria/utils";
 import {focusSafely as $ehFet$focusSafely} from "@react-aria/interactions";
 import {useOverlayTriggerState as $ehFet$useOverlayTriggerState} from "react-stately";
 import $ehFet$react, {createContext as $ehFet$createContext, forwardRef as $ehFet$forwardRef, useContext as $ehFet$useContext, useRef as $ehFet$useRef, useState as $ehFet$useState, useEffect as $ehFet$useEffect, useMemo as $ehFet$useMemo} from "react";
@@ -107,7 +107,7 @@ function $07b14b47974efb58$var$PopoverInner({ state: state, isExiting: isExiting
     ]);
     // Focus the popover itself on mount, unless a child element is already focused.
     (0, $ehFet$useEffect)(()=>{
-        if (isDialog && ref.current && !ref.current.contains(document.activeElement)) (0, $ehFet$focusSafely)(ref.current);
+        if (isDialog && ref.current && !nodeContains(ref.current, document.activeElement)) (0, $ehFet$focusSafely)(ref.current);
     }, [
         isDialog,
         ref

🔦 Context

We are using a microfrontend architecture where a react application is loaded into a web component. This application includes a menu and when that menu is clicked, it doesn't do anything. This web component has a Shadow DOM for encapsulating CSS styling.

We're using UNSAFE_PortalProvider to ensure that the portal is also inside of the web component and Shadow DOM. This is necessary as the Menu needs styling that is only available in the web component.

We found this issue which is related but to me it seemed different enough create a new issue: https://github.com/adobe/react-spectrum/issues/6133

🖥️ Steps to Reproduce

Reproduction Repo link

https://github.com/johnpangalos/rac-shadow-dom-bug-report

Reproduction Steps

1. Create a simple web component with Shadow DOM
2. Use the web component as a root for a React app
3. Use the UNSAFE_PortalProvider to add a portal inside the web component
4. Add a Menu (or any popover) to your app
5. Try to click on a menu item - the popover will close instead of executing the action

React Setup (main.tsx)

The React application is mounted inside the Shadow DOM with:

1. Shadow DOM enablement: enableShadowDOM() from @react-stately/flags
2. CSS injection: Stylesheet is manually added to Shadow DOM
3. Portal setup: A dedicated portal container is created inside the Shadow DOM
4. Portal provider: UNSAFE_PortalProvider wraps the app, targeting the shadow DOM portal

const portal = document.createElement("div");
portal.id = "shadow-dom-portal";
root.shadowRoot?.appendChild(portal);

<UNSAFE_PortalProvider getContainer={() => portal}>
  <App />
</UNSAFE_PortalProvider>;

<MenuTrigger>
  <Button>Menu Button</Button>
  <Popover>
    <Menu onAction={(id) => alert(id)}>
      <MenuItem id="new">New…</MenuItem>
      <MenuItem id="open">Open…</MenuItem>
      <!-- More menu items -->
    </Menu>
  </Popover>
</MenuTrigger>

Version

react-aria: 3.42.0 react-aria-components: 1.11.0

What browsers are you seeing the problem on?

Chrome, Firefox, Safari

If other, please specify.

No response

What operating system are you using?

MacOS 15.5

🧢 Your Company/Team

ThomsonReuters / Pagero

🕷 Tracking Issue

No response

[LFDanLu]

Thanks for the issue, our ShadowDOM support is tenuous at best so its helpful to get these reports! I was unable to run your sample repo due to a crypto.hash error in one of the dependencies, but looking at your applied changes it seems it was primarily because there were places where we didn't substitute .contain calls with our nodeContains shadowDOM helper.

@snowystinger I forget some of the details around https://github.com/adobe/react-spectrum/pull/6046 where these helpers were introduced, but I seem to recall at one point that the goal was to basically use those helpers at any time we would ordinarily do document.activeElement/.contains/event.target. Do you remember if there were any reasons we didn't do that for the above files (FocusScope/etc) or maybe it was just something we missed?

[snowystinger]

Thanks for the patch, doesn't look too involved to change our code. I came across some of these while reviewing https://github.com/adobe/react-spectrum/pull/7751#pullrequestreview-3083364830 the other day. As far as I've made it, the change from node.contains to nodeContains in these locations is correct. I'll have to make a PR to review it more.

The biggest help would be tests to work against. This is important for contributors to provide if at all possible because there are so many different setups possible. I'd like to make sure that we are actually meeting the needs of users.

There are examples of shadow dom setups in https://github.com/adobe/react-spectrum/blob/155e361c6e2910c0b087ccba7cc61541fb50b0a7/packages/%40react-aria/focus/test/FocusScope.test.js#L1981

[johnpangalos]

Hey, thanks for taking the time to get back to me.

was unable to run your sample repo due to a crypto.hash error in one of the dependencies, but looking at your applied changes it seems it was primarily because there were places where we didn't substitute .contain calls with our nodeContains shadowDOM helper.

Strange, I'll take a look and see if I can fix that. Not sure what's happening there.

The biggest help would be tests to work against. This is important for contributors to provide if at all possible because there are so many different setups possible.

I can take a stab at adding a few tests, what would be the best way to get these tests to you? Just open a PR with non passing tests?

[snowystinger]

yep, if you feel like it, you can add the code from your patch as well so they pass. but I'm also happy just to have a PR open with failing tests that I can use

Thank you very much

[pzaczkiewicz-athenahealth]

I've run into this issue as well, and is complicating our adoption of react-aria-components because we use the shadow DOM extensively. The changes in https://github.com/adobe/react-spectrum/pull/8806 look good, but I agree that the tests need work, and the PR has been stalled for a month. Is the best way to move this forward to make my own fork and make a new PR? I can't guarantee that I'll be any faster than @johnpangalos, as I have competing priorities. Still, I wanted to get the conversation started, as I haven't made any contributions here yet, but I really like the library.

[johnpangalos]

I don't mind at all :D Sorry that I've stalled on it.

[pzaczkiewicz-athenahealth]

No worries, as I said, I too have competing priorities. Part of the tension in my head right now is that I'm worried if a global find/replace of s/(\w+)\.target/getEventTarget($1)/, s/([\w.()\[\]]+)\??\.contains\(([^)]+)\)/nodeContains($1, $2)/, and s/([\w.]+)\.activeElement\b/getActiveElement($1)/ isn't done, then we'll just keep finding more targets, contains and activeElement that need conversion as more react-spectrum components are used within the Shadow Dom under different circumstances. If something isn't done systematically, then shadow-dom compatibility will be a long game of whack-a-mole.

At the same time, the current state of my PR is touching 64 files. I got Typescript happy within VSCode for the files I had open, but that's way too much scope for me to validate or test, especially as an outsider to the project.

[pzaczkiewicz-athenahealth]

@LFDanLu's comment on August 6 implies that my systematic approach would be welcome. I could probably harden the tests that @johnpangalos added, but I won't be able to add shadow-dom tests to everything. I'll be reliant on Typescript and existing tests to confirm that I haven't broken things for the flag-off case.

@snowystinger, do you have advice?

[lawctan]

is there a plan to fix this in near term as well as other shadow dom related issues on other components? shadow doms are used extensively on chrome extensions so this would be super useful

[snowystinger]

Due to our current priorities, I think it likely we'll get to the shadow dom work in the new year. Thank you for your patience and understanding.