[all components] Fix canceled exit unmount

[atomiks]

Fixes #5395

Changes

• Finish exit cleanup when a canceled animation has no replacement animation.
• Add coverage for switching between animated tooltips across a trigger gap.

[pkg-pr-new]

• base-ui-tanstack-start

• vite-css-base-ui-example

  pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/react@5401
  

  pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/utils@5401
  

commit: 5e14387

[code-infra-dashboard]

Bundle size

Bundle	Parsed size	Gzip size
@base-ui/react	🔺+71B(+0.02%)	▼-8B(-0.01%)

Details of bundle changes

Performance

Total duration: 991.64 ms -53.14 ms^(-5.1%) | Renders: 78 ⁽⁺⁰⁾ | Paint: 1,524.47 ms -116.43 ms^(-7.1%)

No significant changes — details

---

Check out the code infra dashboard for more information about this PR.

[netlify]

✅ Deploy Preview for base-ui ready!

Name	Link
🔨 Latest commit	5e14387
🔍 Latest deploy log	https://app.netlify.com/projects/base-ui/deploys/6a6ff263bab1420008ed3eae
😎 Deploy Preview	https://deploy-preview-5401--base-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[atomiks]

@claude review

[github-actions]

PR review

The core change is right: the aborted-animation branch previously had no terminal state, so a canceled exit transition with no replacement animation left the cleanup callback permanently unfired (the stuck tooltip in #5395). Nothing here is merge-blocking, but the new terminal done() makes every stale, in-flight promise chain fire instead of silently dying — and usePopupViewport is the one call site that passes no AbortSignal, so it can now run a stale cleanup that cuts a viewport morph short. The added test is a genuine regression test (verified it reaches the new branch and would time out without the fix).

Bugs (2)

1. 🟠 Stale viewport cleanup can now fire and abort an in-flight morph

Location: packages/react/src/utils/usePopupViewport.tsx:164 (mechanism at packages/react/src/internals/useAnimationsFinished.ts:89)

onAnimationsFinished(() => {
  setPreviousContentNode(null);
  setPreviousContentDimensions(null);
  capturedNodeRef.current = null;
});

All seven useAnimationsFinished call sites pass treatAbortedAsFinished = false, so all of them now reach the new terminal done(). Six of them pass an AbortSignal that their effect cleanup aborts (useOpenChangeComplete.tsx:22, usePopupAutoResize.ts:129, useCollapsiblePanel.ts:329, NavigationMenuTrigger.tsx:183, PopoverPositioner.tsx:115, MenuPositioner.tsx:248). usePopupViewport.tsx:164 passes none, and the hook has no other way to stop an already-started chain — frame.cancel() at useAnimationsFinished.ts:36 only cancels a rAF that has not fired yet, not a pending Promise.all.

Before this diff a stale chain whose element had no live animations simply died in the empty-array branch. Now it calls done(), which flushSynces the cleanup belonging to a previous transition.

Failure scenario: Tooltip.Viewport / Popover.Viewport / Menu.Viewport / PreviewCard.Viewport with a morph transition on the viewport content. User activates trigger T1, then T2 (T1→T2 morph starts and exec() begins watching the data-current div), then T3 ~150 ms later. React remounts/unmounts the watched data-current node, its transitions are canceled, getAnimations() on the now-detached node returns [], .some(...) is false, and the T1→T2 callback fires — clearing previousContentNode / previousContentDimensions / capturedNodeRef that the live T2→T3 transition just set. The T2→T3 morph then plays with no previous-content snapshot and data-transitioning flips off mid-animation.

Fix: Thread an AbortController through the useIsoLayoutEffect at usePopupViewport.tsx:141 the way the other six call sites do — pass its signal to onAnimationsFinished and abort it in the effect cleanup (this also disconnects the data-starting-style MutationObserver, which currently leaks because signal?.addEventListener is a no-op with null).

2. 🟡 done() fires immediately when a replacement transition registers one frame late

Location: packages/react/src/internals/useAnimationsFinished.ts:76-89

const currentAnimations = resolvedElement.getAnimations();

if (currentAnimations.some((a) => a.pending || a.playState !== 'finished')) {
  exec();
  return;
}

done();

The re-check runs synchronously in the rejection microtask. The codebase already documents that this snapshot can be empty even though an animation is about to start — see useCollapsiblePanel.ts:299-302: "Chrome can still register the exit transition one frame later when an Accordion closes one item while opening another, so wait one frame before watching animations." In that window the old code hung (bad) and the new code completes early (also wrong, just differently): the consumer callback runs while the element is about to animate.

Failure scenario: Accordion closes one item while opening another, or NavigationMenuTrigger.scheduleAutoSizeReset (NavigationMenuTrigger.tsx:179) watching a size transition that is canceled and re-created a frame later — setAutoSizes(popup) runs immediately, snapping the popup to content size instead of animating. Similarly PopoverPositioner.tsx:112 / MenuPositioner.tsx would set instantType: 'trigger-change' mid-move, killing the trigger-to-trigger animation.

Fix: Before the terminal done(), wait one animation frame and re-check resolvedElement.getAnimations() (reusing the existing frame handle), so a late-registered replacement animation is picked up instead of being missed.

3. ℹ️ The .catch() → two-argument .then() change is load-bearing, not cosmetic

Location: packages/react/src/internals/useAnimationsFinished.ts:60-66

Worth calling out for reviewers skimming this as a reformat: with the old .then(...).catch(...) shape, a throw inside the success handler's done() (i.e. flushSync(fnToExecute)) fell into the same catch, which — with the new terminal done() — would have invoked the consumer callback a second time. The two-argument form is what prevents that. The observable trade-off is that such an error is now an unhandled rejection instead of being swallowed; that seems preferable, but it is an undocumented behavior change.

Tests (1)

1. 🟡 No coverage for the complementary "replacement animation exists" path

Location: packages/react/src/tooltip/root/TooltipRoot.test.tsx:680

The new test pins the canceled-with-no-replacement case, but nothing pins the other half of the same branch: that a canceled animation with a pending replacement still waits rather than completing early. Since done() is now the fall-through, deleting or weakening the currentAnimations.some(...) guard would make popups unmount mid-transition and every existing test would still pass.

Failure scenario: A future refactor simplifies the rejection handler to always done(); a popup whose exit transition is canceled and immediately re-created (property-change cancellation, the exact case the comment at useAnimationsFinished.ts:83-84 describes) unmounts without animating, and CI is green.

Fix: Add a sibling case where the exiting popup's transition is canceled but a replacement animation starts, and assert the popup stays mounted until that replacement finishes. Also consider asserting the intermediate firstPopup.dataset.instant === 'delay' in the new test — today a regression in the instantType wiring (TooltipRoot.tsx:100-116) surfaces only as an opaque waitFor timeout on the final unmount assertion.

Simplifications (1)

1. 🟡 treatAbortedAsFinished is dead — no call site passes true

Location: packages/react/src/internals/useAnimationsFinished.ts:19,71-74

treatAbortedAsFinished = true,
...
if (treatAbortedAsFinished) {
  done();
  return;
}

All seven call sites in the repo explicitly pass false, and the hook lives under internals/ with no public re-export, so the true default and its branch are unreachable. This PR narrows the gap further — the false path now also completes on abort when nothing replaced the animation, so the only remaining difference is the one retry.

Failure scenario: Every consumer ships a parameter, a default, a branch, and a JSDoc block that nothing can reach — dead bytes in a library where bundle size is a top constraint, plus a stale mental model for the next maintainer.

Fix: Drop the third parameter and the if (treatAbortedAsFinished) block, and update the seven call sites to two arguments.

Docs (1)

1. 🟡 treatAbortedAsFinished JSDoc now describes behavior the hook no longer has

Location: packages/react/src/internals/useAnimationsFinished.ts:12-13

* @param treatAbortedAsFinished - Whether to treat aborted animations as finished. If `false`, and there are aborted animations,
*   the function will check again if any new animations have started and wait for them to finish.

After this change, false also treats aborted animations as finished whenever no replacement animation is pending or unfinished.

Failure scenario: A maintainer adding a new call site reads this and assumes the callback is suppressed on cancellation — either reintroducing the hang this PR fixes, or missing the premature-completion risk described in Bugs #1 and #2.

Fix: Reword to something like: "If false, aborted animations trigger a re-check: the callback waits for any newly started animations, and runs immediately if none were registered." (If the parameter is removed per Simplifications #1, fold this into the hook's top-level description instead.)

Verdict

Approve after nits — the fix addresses a real hang and the regression test is sound; the missing AbortSignal in usePopupViewport is the one thing worth handling in this PR, since the new terminal done() is what makes that stale chain observable.

---

🤖 Review generated with Claude Code

---

🤖 Review generated with Claude Code · medium effort · 25 turns · 15m0s · $4.72 · run

[atomiks]

Note

GPT-5.6 finds this bug: useAnimationsFinished can complete against a stale element when its ref moves to an animated replacement. Ignored intentionally as too niche.