radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.37k stars 767 forks source link

Accessible id collisions when using React < 18, multiple bundles #2576

Open zacowan opened 9 months ago

zacowan commented 9 months ago

Bug report

For the current/expected behavior, assume the following:

  1. The React version is < 18, so useId relies on a global counter for ids.
  2. Two components rendered by two separate JavaScript bundles are rendered on the same page at the same time using the Radix Dialog primitive.

Current Behavior

Under the described assumptions, each bundle has no knowledge that the other bundle exists. So, when the Radix Dialog primitive gets rendered, each bundle increments its own counter separately from the other bundle. Consider the following two bundles:

// Bundle A
import * as Dialog from '@radix-ui/react-dialog';

export default () => (
  <Dialog.Root>
    <Dialog.Trigger>
      TRIGGER_BUNDLE_A
    </Dialog.Trigger>
    <Dialog.Portal>
      <Dialog.Overlay />
      <Dialog.Content>
        <Dialog.Title>
          TITLE_BUNDLE_A
        </Dialog.Title>
        <Dialog.Description>
          DESCRIPTION_BUNDLE_A
        </Dialog.Description>
        <Dialog.Close>
          CLOSE_BUNDLE_A
        </Dialog.Close>
      </Dialog.Content>
    </Dialog.Portal>
  </Dialog.Root>
);
// Bundle B
import * as Dialog from '@radix-ui/react-dialog';

export default () => (
  <Dialog.Root>
    <Dialog.Trigger>
      TRIGGER_BUNDLE_B
    </Dialog.Trigger>
    <Dialog.Portal>
      <Dialog.Overlay />
      <Dialog.Content>
        <Dialog.Title>
          TITLE_BUNDLE_B
        </Dialog.Title>
        <Dialog.Description>
          DESCRIPTION_BUNDLE_B
        </Dialog.Description>
        <Dialog.Close>
          CLOSE_BUNDLE_B
        </Dialog.Close>
      </Dialog.Content>
    </Dialog.Portal>
  </Dialog.Root>
);

If each bundle is loaded onto the same page separately from each other, the contentId, titleId, and descriptionId values will be radix-0, radix-1, and radix-2, respectively, for each bundle, resulting in an id collision. This, in turn, breaks the aria-attributes necessary to ensure that the Dialog primitive remains accessible.  

Expected behavior

To be fair, current behavior could be considered the expected behavior. However, it would be nice to have an option to enable Radix primitives to remain accessible, even if they are from different bundles.

Reproducible example

Unable to reproduce easily in CodeSandbox, as I'm not entirely sure how to execute two separate bundles in a CodeSandbox environment.

Suggested solution

I would be open to implementing any of these proposals (or others) and validating, if that would be helpful 😄

To solve this, I have two initial proposals:

Utilize the Window Object for the Count

This is a more automated solution, and involves changing the implementation of useId to something like this:

  useLayoutEffect(() => {
    if (!deterministicId) {
      if (!window['radix-use-id-count']) window['radix-use-id-count'] = 0;
      setId((reactId) => reactId ?? String(window['radix-use-id-count']++));
    }
  }, [deterministicId]);
  return deterministicId || (id ? `radix-${id}` : '');
}

However, I'm not sure if this may result in race conditions between two separate bundles (or even two components being rendered from the same bundle) and the window object.

Provide Explicit Props to Allow for Deterministic Ids

This is a more manual approach and perhaps clutters the minimal API of some of these primitives but is rather simple: add props to the Root of primitives that use useId to allow for passing a deterministicId to the underlying useId hook. There are several primitives that use useId, but for the Dialog primitive that might look something like this:

interface DialogProps {
  children?: React.ReactNode;
  open?: boolean;
  defaultOpen?: boolean;
  onOpenChange?(open: boolean): void;
  modal?: boolean;
  contentId?: string;
  titleId?: string;
  descriptionId?: string;
}
// ...
<DialogProvider
      contentId={useId(contentId)}
      titleId={useId(titleId)}
      descriptionId={useId(descriptionId)}
    >
      {children}
</DialogProvider>

This has additional benefits beyond just the "we can provide an explicit id and it will work". Providing the option to pass an explicit id will let users use the deterministic ids for selectors in their css classes (albeit this may not be useful for the majority of users).

Additional context

Your environment

Software Name(s) Version
Radix Package(s) @radix-ui/react-dialog 1.0.5
React n/a ^17
Browser Microsoft Edge 119.0.2151.97
Assistive tech n/a n/a
Node Node.js 18.17.0
npm/yarn npm 9.6.7
Operating System macOS Ventura 13.6
benoitgrelard commented 6 months ago

This seems like a really niche issue 🤔 Do you have a practical example of where this would be relevant?

zacowan commented 6 months ago

This seems like a really niche issue 🤔 Do you have a practical example of where this would be relevant?

Using two JavaScript bundles on the same page that are loaded separately is a common pattern for micro-frontends - so a practical example of where this might be useful is in any micro-frontend environment.

But, this is definitely a niche issue 😅

marcosss3 commented 5 months ago

@benoitgrelard this issue is currently preventing us from locating a modal by role and its accessible name with testing libraries (e.g. Playwright), since there's another Radix primitive in the page that uses the same id value.