You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
useConfirm() / confirm() renders the ConfirmationDialog into its own detached createRoot attached directly to document.body, outside of the calling component's React tree. Because this root is a sibling of the app's root rather than a descendant, it is invisible to anything that manages the app tree — most notably @testing-library/react's cleanup(), which only unmounts roots it created via render().
The practical consequence: if a confirm() promise is left unresolved when a test finishes (a very common situation — a test renders a component, triggers the dialog, asserts, and ends without dismissing it; or a retried/failed test bails mid-flow), the dialog's backdrop leaks into the next test, intercepts pointer events, and cascades unrelated failures (kebab menus won't open, queries find duplicate elements, etc.). Consumers cannot clean this up from their own teardown without DOM-scraping hacks, because the root is not reachable from their tree.
lethostElement: Element|null=nullasyncfunctionconfirm(options: ConfirmOptions): Promise<boolean>{const{content, ...confirmationDialogProps}=optionsreturnnewPromise(resolve=>{hostElement||=document.createElement('div')if(!hostElement.isConnected)document.body.append(hostElement)constroot=createRoot(hostElement)// ← detached sibling rootconstonClose: ConfirmationDialogProps['onClose']=gesture=>{root.unmount()// ← unmounts root, but leaves hostElement on <body>resolve(gesture==='confirm')}root.render(<BaseStyles><ConfirmationDialog{...confirmationDialogProps}onClose={onClose}>{content}</ConfirmationDialog></BaseStyles>,)})}
This is the natural follow-on to #2990 — that issue swapped the deprecated ReactDOM.render for createRoot, but kept the detached-root architecture. #2990's author actually proposed the in-tree usePortal approach below before the simpler createRoot swap shipped.
Why it leaks to consumers
Not unmounted by tree teardown.createRoot(hostElement) with hostElement appended to document.body is a second React root. Testing-library cleanup(), React tree unmounts, and provider teardown all operate on the app tree and never touch it. There is no supported way for a consumer to dismiss a leaked confirm dialog.
hostElement is never removed. Even on the happy path, onClose calls root.unmount() but leaves the now-empty <div> attached to <body> permanently. It accumulates across calls.
No teardown handle. There is no exported way to reset/close the active confirm dialog (e.g. for test teardown or route changes).
Impact
This is causing flaky browser tests in github/github-ui. Any test that renders a component using useConfirm (or directly rendering ConfirmationDialog) and ends without dismissing the dialog poisons subsequent tests in the same file/worker. Our current mitigation is a global afterEach that presses Escape and, as a last resort, force-detaches any [role="alertdialog"] host from <body> — a workaround that should not be necessary and that every Primer consumer would otherwise have to reinvent.
Proposed fixes (in rough order of value)
Render confirm in-tree via a provider + portal instead of a detached createRoot. A ConfirmationDialogProvider (or a usePortal-style hook, as floated in useConfirm calls deprecated ReactDOM.render, triggering warning in React 18 #2990) that renders the active dialog inside the app tree means it unmounts with the tree, so cleanup() and normal unmount handle it automatically and the entire class of leak disappears. This is the real fix (behavioral change; likely a major-release item).
Expose a teardown handle in the interim — e.g. an exported __closeConfirm() / resetConfirm(), or making the active root/host discoverable — so consumers can deterministically dismiss in tests/route changes without scraping the DOM.
Remove hostElement from <body> on close. At minimum, onClose should detach hostElement after root.unmount() so the empty host doesn't linger and accumulate.
Validation: is anything else affected?
I scanned @primer/react@38.26.0 (and re-checked main): ConfirmationDialog.tsx is the only component that creates a detached React root. It is the sole createRoot call site and the sole manual document.body.append(host) outside of Portal. Portal uses createPortal into an in-tree (or registered) container, so it unmounts with the tree and is not affected. So this issue is scoped to useConfirm/confirm.
Environment
@primer/react: 38.26.0 (also confirmed present on main)
React 18+ (createRoot)
Reproduces in @testing-library/react browser/jsdom test environments
Summary
useConfirm()/confirm()renders theConfirmationDialoginto its own detachedcreateRootattached directly todocument.body, outside of the calling component's React tree. Because this root is a sibling of the app's root rather than a descendant, it is invisible to anything that manages the app tree — most notably@testing-library/react'scleanup(), which only unmounts roots it created viarender().The practical consequence: if a
confirm()promise is left unresolved when a test finishes (a very common situation — a test renders a component, triggers the dialog, asserts, and ends without dismissing it; or a retried/failed test bails mid-flow), the dialog's backdrop leaks into the next test, intercepts pointer events, and cascades unrelated failures (kebab menus won't open, queries find duplicate elements, etc.). Consumers cannot clean this up from their own teardown without DOM-scraping hacks, because the root is not reachable from their tree.Source (current
main):https://github.com/primer/react/blob/main/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx
This is the natural follow-on to #2990 — that issue swapped the deprecated
ReactDOM.renderforcreateRoot, but kept the detached-root architecture. #2990's author actually proposed the in-treeusePortalapproach below before the simplercreateRootswap shipped.Why it leaks to consumers
createRoot(hostElement)withhostElementappended todocument.bodyis a second React root. Testing-librarycleanup(), React tree unmounts, and provider teardown all operate on the app tree and never touch it. There is no supported way for a consumer to dismiss a leaked confirm dialog.hostElementis never removed. Even on the happy path,onClosecallsroot.unmount()but leaves the now-empty<div>attached to<body>permanently. It accumulates across calls.Impact
This is causing flaky browser tests in
github/github-ui. Any test that renders a component usinguseConfirm(or directly renderingConfirmationDialog) and ends without dismissing the dialog poisons subsequent tests in the same file/worker. Our current mitigation is a globalafterEachthat pressesEscapeand, as a last resort, force-detaches any[role="alertdialog"]host from<body>— a workaround that should not be necessary and that every Primer consumer would otherwise have to reinvent.Proposed fixes (in rough order of value)
confirmin-tree via a provider + portal instead of a detachedcreateRoot. AConfirmationDialogProvider(or ausePortal-style hook, as floated inuseConfirmcalls deprecatedReactDOM.render, triggering warning in React 18 #2990) that renders the active dialog inside the app tree means it unmounts with the tree, socleanup()and normal unmount handle it automatically and the entire class of leak disappears. This is the real fix (behavioral change; likely a major-release item).__closeConfirm()/resetConfirm(), or making the active root/host discoverable — so consumers can deterministically dismiss in tests/route changes without scraping the DOM.hostElementfrom<body>on close. At minimum,onCloseshould detachhostElementafterroot.unmount()so the empty host doesn't linger and accumulate.Validation: is anything else affected?
I scanned
@primer/react@38.26.0(and re-checkedmain):ConfirmationDialog.tsxis the only component that creates a detached React root. It is the solecreateRootcall site and the sole manualdocument.body.append(host)outside ofPortal.PortalusescreatePortalinto an in-tree (or registered) container, so it unmounts with the tree and is not affected. So this issue is scoped touseConfirm/confirm.Environment
@primer/react: 38.26.0 (also confirmed present onmain)createRoot)@testing-library/reactbrowser/jsdom test environments