Skip to content

CI flake: leaked Mantine transition timer fails the whole run (env="test" does not suppress it, contrary to AGENTS.md) #1984

Description

@cliffhall

Symptom

CI fails the coverage step with every test passing:

Test Files  318 passed (318)
Tests       4929 passed (4929)
Errors      1 error          ← fails the step

⎯⎯ Uncaught Exception ⎯⎯
ReferenceError: window is not defined
 ❯ resolveUpdatePriority  react-dom-client.development.js:1308:7
 ❯ dispatchSetState       react-dom-client.development.js:9126:14
 ❯ Timeout._onTimeout     @mantine/core/esm/components/Transition/use-transition.mjs:61:13
This error originated in "src/components/groups/ServerRemoveConfirmModal/ServerRemoveConfirmModal.test.tsx"

Observed twice so far, both on unrelated docs-only branches, both attributed to the same file:

Run Branch
31559868333 v2/docs/1979-dco-signoff
31526763495 v2/chore/1977-isolate-web-smoke-catalog

This is the #1760 class: a Mantine Transition timer firing after happy-dom has disposed the file's window. It fails the entire run from an arbitrary file, so it reads as an unrelated failure and costs a re-run.

Root cause — the documented mitigation does not do what we think

Both AGENTS.md and the comment in src/test/setup.ts assert that env="test" makes Mantine transitions synchronous and is therefore "the actual protection against the #1760 post-teardown leak". It isn't.

env is consulted only in Transition.mjs, at the render branch:

// components/Transition/Transition.mjs:45
if (transitionDuration === 0 || env === "test") { /* render children directly */ }

But useTransition() is called at line 32, before that check — hooks cannot be conditional — and its useDidUpdate(…, [mounted]) still runs the timer path:

// components/Transition/use-transition.mjs
const reduceMotion = theme.respectReducedMotion ? shouldReduceMotion : false;
const newTransitionDuration = reduceMotion ? 0 : shouldMount ? duration : exitDuration;
if (newTransitionDuration === 0) { /* synchronous */ }
else {
  rafRef.current = requestAnimationFrame(() => { … 
    transitionTimeoutRef.current = window.setTimeout(…, newTransitionDuration);  // ← leaks
  });
}

So env="test" skips the visual transition while the hook still schedules real timers.

Measured, rendering a bare <Modal> through renderWithMantine and toggling opened:

scheduledTimeouts: [200, 200, 200]

Three real 200ms window.setTimeouts per open, under the configuration we believe suppresses them. Any test that toggles a Modal/Transition leaves those pending; if its environment is disposed first, they throw. ServerRemoveConfirmModal.test.tsx toggles opened across 9 renders, which is consistent with it being implicated both times.

Note the second half of the setup.ts comment already derives most of this — matchMedia is pinned to prefers-reduced-motion: reduce, and it correctly notes Mantine 8 defaults theme.respectReducedMotion to false so that pin does nothing on its own. Only its concluding sentence ("env="test" … forces transitions synchronous regardless") is wrong.

Partial fix, measured — not a cure

Since matchMedia already reports reduced motion, setting respectReducedMotion: true on the test theme should zero the duration. It helps but does not eliminate the leak:

Configuration Timers scheduled
current (env="test") [200, 200, 200]
+ respectReducedMotion: true [200]

One 200ms timer survives, from a path not yet identified (Modal overlay / scroll-lock / a nested Transition are the candidates). Landing respectReducedMotion alone would reduce the flake rate and make it rarer and harder to diagnose, without fixing it — so it should not be applied by itself.

Suggested work

  1. Find the remaining timer's source (instrument window.setTimeout and capture a stack).
  2. Make transitions genuinely timer-free under test, or add an afterEach that drains/clears pending Mantine timers before environment disposal.
  3. Correct AGENTS.md and the setup.ts comment — they currently tell contributors that a protection exists which does not. That is arguably the most important part: the rule "always render through renderWithMantine" is still good practice, but it is not the leak guard it is advertised as.
  4. Consider failing louder: the step is named "Enforce per-file coverage gate", so an unhandled-error failure with 100% passing tests reads as a coverage problem and sends the reader to the wrong place.

Not reproducible locally

Six consecutive local npm run ci runs were green, and forcing ViewHeader.test.tsx's settleMs to 1 did not reproduce it across the full unit project either. It appears to need CI-like load, where a file's environment is disposed before a pending 200ms timer fires.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingv2Issues and PRs for v2

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions