From ec2ad3433ded8945f963409ddcd45a4d0d819a4d Mon Sep 17 00:00:00 2001 From: Jon Laing Date: Mon, 3 Aug 2026 10:04:27 -0400 Subject: [PATCH] fix(dom): wait for element connection before starting enter animation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit forkSlotEnter forks the enter animation fiber via Effect.forkIn(slotScope) from inside addSlot. Effect's scheduler can hand that fiber control on the next microtask, before the outer synchronous render flow has finished appending the wrapper's ancestor chain to the document. When that happens, onBeforeEnter runs against a disconnected node — getComputedStyle returns empty strings, browsers won't compute or transition styles against detached nodes, and the enter transition never fires. The animation stalls to the timeout. Hydration didn't surface this: it walks pre-existing DOM so every node was already connected when the fiber ran. Client-mode re-mounts (e.g. router nav-back) broke it, but only when the animated block sat inside another wrapper — its ancestor had to be appended AFTER the fork ran. Reproduced with a top-level `when` toggle wrapping two levels of `$.div` around an `animated` — element.isConnected was false at onBeforeEnter on the second mount. Yield microtasks at the start of the animation fiber's body until the element is connected, up to 3 attempts. The outer flow completes within 1-2 microtasks in practice; the bound keeps tests that yield an animated element without ever appending it (e.g. the existing "gates on an AnimationGroup" test) from hanging. Co-Authored-By: Claude Opus 4.7 --- .changeset/fix-enter-anim-detached-element.md | 33 +++++++++ packages/dom/src/Control/Control.test.ts | 71 +++++++++++++++++++ packages/dom/src/Control/slotAnimation.ts | 21 ++++++ 3 files changed, 125 insertions(+) create mode 100644 .changeset/fix-enter-anim-detached-element.md diff --git a/.changeset/fix-enter-anim-detached-element.md b/.changeset/fix-enter-anim-detached-element.md new file mode 100644 index 0000000..ecdd6a5 --- /dev/null +++ b/.changeset/fix-enter-anim-detached-element.md @@ -0,0 +1,33 @@ +--- +"@effex/dom": patch +--- + +Fix enter animations firing against a detached element on nested +client-mode re-mounts (e.g. router nav-back on pages with animations +deep inside the route's tree). + +`forkSlotEnter` forks the animation fiber via `Effect.forkIn(slotScope)` +from inside `addSlot`. Effect's scheduler can hand that fiber control +on the next microtask, before the outer synchronous render flow has +finished appending the wrapper's ancestor chain into the document. +When that happens, `onBeforeEnter` fires against a node whose ancestors +aren't yet in the DOM — `getComputedStyle` returns empty strings on +disconnected nodes, and browsers won't compute or transition styles +against them, so the enter transition never fires and the animation +stalls to the timeout. + +On first render this didn't surface because hydration walks pre-existing +DOM: every element was already connected when the fiber ran. It only +broke on subsequent client-mode mounts, and only when the animated +block sat inside another wrapper element (its own ancestor had to be +appended AFTER the fork). + +The animation fiber now yields microtasks until the element is +connected, up to a small bound. In practice the outer flow completes +within one or two microtasks; the bound ensures callers that never +insert their result (e.g. tests that yield an animated element without +appending it to the document) still make progress instead of hanging. + +Regression test in `Control.test.ts` asserts `element.isConnected` is +true at `onBeforeEnter` on both the initial mount AND the toggle-back +of a `when`-nested animated block that lives two wrappers deep. diff --git a/packages/dom/src/Control/Control.test.ts b/packages/dom/src/Control/Control.test.ts index 77148d9..4961e5f 100644 --- a/packages/dom/src/Control/Control.test.ts +++ b/packages/dom/src/Control/Control.test.ts @@ -817,6 +817,77 @@ describe("Control", () => { }).pipe(Effect.provide(TestLayer)), ); + it.scopedLive( + "onBeforeEnter fires against an attached element (nested remount)", + () => + // Regression: on a nested-`animated` re-mount, the enter fiber + // was forked from inside the inner addSlot while the ancestor + // chain was still being assembled bottom-up in memory. Effect's + // scheduler could give the fiber control on the next microtask, + // before the outer flow appended the wrapper into the document. + // onBeforeEnter then fired against a detached node — + // getComputedStyle returned empty strings and the transition + // never fired. Verify the element is `isConnected` at onBeforeEnter. + Effect.gen(function* () { + const root = document.createElement("div"); + document.body.appendChild(root); + + const visible = yield* Signal.make(true); + const connectedFlags: boolean[] = []; + + // Two levels of nesting under the `when` — mirrors the shape + // of Outlet → HomePage → Headline → animated in the report. + const view = yield* when(visible, { + onTrue: () => + $.div( + { class: "outer" }, + $.div( + { class: "middle" }, + animated( + { + animate: { + enterFrom: "opacity-0", + enter: "transition-opacity duration-100", + enterTo: "opacity-100", + onBeforeEnter: (el) => + Effect.tap(el, (e) => + Effect.sync(() => { + connectedFlags.push(e.isConnected); + }), + ), + timeout: 20, + }, + }, + () => $.div({ class: "target" }, $.of("Hi")), + ), + ), + ), + onFalse: () => $.div({ class: "gone" }), + }); + + // Attach the top-level result under a real DOM root so + // `isConnected` propagates all the way down. + root.appendChild(view as HTMLElement); + + // First mount — everything attached synchronously above. + yield* Effect.sleep("40 millis"); + expect(connectedFlags.length).toBe(1); + expect(connectedFlags[0]).toBe(true); + + // Toggle away and back — this is the failure shape from the + // portfolio report. + yield* visible.set(false); + yield* Effect.sleep("10 millis"); + yield* visible.set(true); + yield* Effect.sleep("40 millis"); + + expect(connectedFlags.length).toBe(2); + expect(connectedFlags[1]).toBe(true); + + document.body.removeChild(root); + }).pipe(Effect.provide(TestLayer)), + ); + it.scopedLive("gates on an AnimationGroup", () => Effect.gen(function* () { // Two `animated` blocks in sequence — the second must not fire its diff --git a/packages/dom/src/Control/slotAnimation.ts b/packages/dom/src/Control/slotAnimation.ts index cecadd6..4a86d40 100644 --- a/packages/dom/src/Control/slotAnimation.ts +++ b/packages/dom/src/Control/slotAnimation.ts @@ -159,6 +159,27 @@ export const forkSlotEnter = ( if (grp) { yield* _awaitGate(grp); } + // Wait for the element to be attached to the document before the + // enter lifecycle starts. On client-mode re-mount (e.g. a router + // nav-back), the fiber is forked from inside addSlot while the + // ancestor chain is still being assembled bottom-up in memory — + // Effect's scheduler can hand this fiber control on the next + // microtask, before the outer flow appends the wrapper into the + // document. onBeforeEnter would then fire against a detached + // node, getComputedStyle would return empty strings, and the + // transition would never fire (browsers won't compute or + // transition styles on disconnected nodes). + // + // Yield microtasks until the element is connected, up to a small + // bound. In practice the outer flow finishes within one or two + // microtasks; the retry bound guards against callers that never + // insert their result (e.g. tests that yield an animated element + // without appending it to the document) — we proceed after a + // handful of tries even if still detached so the animation + // continues to be best-effort rather than hanging. + for (let i = 0; i < 3 && !element.isConnected; i++) { + yield* Effect.yieldNow(); + } const run = runEnterAnimation(Effect.succeed(element), animate).pipe( Effect.ensuring(grp ? _complete(grp) : Effect.void), );