diff --git a/.changeset/fix-outlet-animation-config.md b/.changeset/fix-outlet-animation-config.md new file mode 100644 index 0000000..770215f --- /dev/null +++ b/.changeset/fix-outlet-animation-config.md @@ -0,0 +1,27 @@ +--- +"@effex/router": patch +"@effex/dom": patch +--- + +fix(router): Outlet now actually applies its animation configuration + +`OutletConfig.animate` was defined in the type but never read in the +implementation — the underlying `reconcile` call passed only +`getTargetKeys` and `renderSlot`, so nothing wired the animation config +through to the control ctx. Consumers configuring `animate` saw abrupt +route transitions regardless of what they set. + +`Outlet` now provides `AnimationConfigCtx` (the same tag `when`/`match`/ +`each` use) via `Effect.provideService`, matching the pattern those +combinators follow. `provideService` uses `provideContext` internally +rather than `provideSomeLayer`'s `scopedWith` — no scope is created and +no finalizer race is introduced (see #78 for the finalizer-race pattern +we're deliberately avoiding). + +Also adds an `intro?: boolean` field to `OutletConfig`, so the initially +matched route can re-animate on hydration in cases like a decorative +opening scene. Same shape as `when`/`match`/`each`/`animated`. + +`AnimationConfigCtx` and `ClientControlCtx` are now re-exported from +`@effex/dom`'s package root (they were exported from +`@effex/dom/Control/index.ts` but not lifted). diff --git a/packages/dom/src/Render/hydrate/hydrate.test.ts b/packages/dom/src/Render/hydrate/hydrate.test.ts index b537019..280a772 100644 --- a/packages/dom/src/Render/hydrate/hydrate.test.ts +++ b/packages/dom/src/Render/hydrate/hydrate.test.ts @@ -312,7 +312,10 @@ describe("Hydration", () => { container.innerHTML = await Effect.runPromise(renderToString(App())); await hydrate(App(), container); - await new Promise((r) => setTimeout(r, 20)); + // Wait long enough that if the fork DID try to run the animation, + // we'd see it — protects the "no re-animate" assertion against + // late-firing regressions. + await new Promise((r) => setTimeout(r, 100)); expect(onBeforeEnter).not.toHaveBeenCalled(); }); @@ -339,8 +342,13 @@ describe("Hydration", () => { container.innerHTML = await Effect.runPromise(renderToString(App())); await hydrate(App(), container); - // Give forked enter animations a tick to invoke the hook. - await new Promise((r) => setTimeout(r, 20)); + // Give forked enter animations time to invoke the hook. Hydration + // animations now wait one requestAnimationFrame before starting + // (see `forkSlotEnter` — needed to give stylesheets a chance to + // apply before forceReflow snapshots the pre-transition state), + // which in jsdom is ~16ms + queue drain. 20ms was too tight on + // slow CI runners. + await new Promise((r) => setTimeout(r, 100)); expect(onBeforeEnter).toHaveBeenCalledTimes(letters().length); }); @@ -366,7 +374,7 @@ describe("Hydration", () => { container.innerHTML = await Effect.runPromise(renderToString(App())); await hydrate(App(), container); - await new Promise((r) => setTimeout(r, 20)); + await new Promise((r) => setTimeout(r, 100)); expect(onBeforeEnter).toHaveBeenCalledTimes(1); }); diff --git a/packages/dom/src/index.ts b/packages/dom/src/index.ts index a8cd514..2c2f3df 100644 --- a/packages/dom/src/index.ts +++ b/packages/dom/src/index.ts @@ -58,6 +58,8 @@ export { matchEither, redraw, animated, + AnimationConfigCtx, + ClientControlCtx, HydrationMismatchError, } from "./Control/index.js"; export type { diff --git a/packages/router/src/Outlet.test.ts b/packages/router/src/Outlet.test.ts new file mode 100644 index 0000000..4e9f81a --- /dev/null +++ b/packages/router/src/Outlet.test.ts @@ -0,0 +1,97 @@ +import { Effect, Layer, Option } from "effect"; +import { beforeEach, describe, expect, it } from "vitest"; + +import { + $, + AnimationConfigCtx, + ClientControlCtx, + DOMRendererLive, +} from "@effex/dom"; + +import { Navigation } from "./Navigation.js"; +import { Outlet } from "./Outlet.js"; +import { Route } from "./Route.js"; +import { concat, empty } from "./Router.js"; + +const TestLayer = Layer.mergeAll(ClientControlCtx, DOMRendererLive); + +describe("Outlet", () => { + beforeEach(() => { + document.body.innerHTML = ""; + }); + + it("provides AnimationConfigCtx when `animate` is set", async () => { + // Route's render function peeks at AnimationConfigCtx to prove the + // Outlet wired it through. Without the fix, `Effect.serviceOption` + // returned `Option.none()` — `config.animate` sat unused in the + // OutletConfig type. + let received: unknown = "not-set"; + + const HomeRoute = Route.make("/").pipe( + Route.render(() => + Effect.gen(function* () { + const cfg = yield* Effect.serviceOption(AnimationConfigCtx); + received = Option.getOrNull(cfg); + return yield* $.div({ class: "home" }, $.of("Home")); + }), + ), + ); + + const router = empty.pipe(concat(HomeRoute)); + const navLayer = Navigation.makeLayer(router, { initialPath: "/" }); + + await Effect.runPromise( + Effect.gen(function* () { + yield* Outlet({ + router, + animate: { + enterFrom: "opacity-0", + enter: "opacity-100", + timeout: 10, + }, + }); + }).pipe( + Effect.scoped, + Effect.provide(navLayer), + Effect.provide(TestLayer), + ), + ); + + expect(received).not.toBeNull(); + expect(received).toMatchObject({ + single: expect.objectContaining({ enterFrom: "opacity-0" }), + }); + }); + + it("does not provide AnimationConfigCtx when no `animate` or `intro` is set", async () => { + // Ensures the wire-up is opt-in: routes that don't configure + // animation see `Option.none()`, so downstream control-ctx code + // treats the slot as non-animated. + let received: unknown = "not-set"; + + const HomeRoute = Route.make("/").pipe( + Route.render(() => + Effect.gen(function* () { + const cfg = yield* Effect.serviceOption(AnimationConfigCtx); + received = Option.getOrNull(cfg); + return yield* $.div({ class: "home" }, $.of("Home")); + }), + ), + ); + + const router = empty.pipe(concat(HomeRoute)); + const navLayer = Navigation.makeLayer(router, { initialPath: "/" }); + + await Effect.runPromise( + Effect.gen(function* () { + yield* Outlet({ router }); + }).pipe( + Effect.scoped, + Effect.provide(navLayer), + Effect.provide(TestLayer), + ), + ); + + expect(received).toBeNull(); + }); +}); diff --git a/packages/router/src/Outlet.ts b/packages/router/src/Outlet.ts index b973693..f144b53 100644 --- a/packages/router/src/Outlet.ts +++ b/packages/router/src/Outlet.ts @@ -1,7 +1,12 @@ -import { Effect, Option, Record } from "effect"; +import { Effect, Option, pipe, Record } from "effect"; import { ControlCtx, reconcile } from "@effex/core"; -import { $, Element, type AnimationOptions } from "@effex/dom"; +import { + $, + AnimationConfigCtx, + Element, + type AnimationOptions, +} from "@effex/dom"; import { buildPath, NavigationContext, type Navigation } from "./Navigation.js"; import { resolveMeta, type Route } from "./Route.js"; @@ -26,6 +31,12 @@ export interface OutletConfig< readonly router: Router; /** Animation options for route transitions */ readonly animate?: AnimationOptions; + /** + * When true, the enter animation also plays for the initially matched + * route on hydration. Default is to attach handlers to the SSR-rendered + * DOM without re-animating. + */ + readonly intro?: boolean; } /** @@ -231,33 +242,45 @@ export const Outlet = < E, R | NavigationContext | ControlCtx > => - Effect.gen(function* () { - const nav = yield* NavigationContext; - const router = config.router; - const layouts = router.layouts; + pipe( + Effect.gen(function* () { + const nav = yield* NavigationContext; + const router = config.router; + const layouts = router.layouts; - // Use pathname as the reconcile key so param-only navigations - // (e.g. /users/alice → /users/bob) trigger a re-render. - return (yield* reconcile(nav.pathname, { - getTargetKeys: (pathname: string) => { - const matched = findMatch(router, pathname); - if (Option.isSome(matched)) return [pathname]; - if (router.fallback) return ["__fallback__"]; - return []; - }, - renderSlot: (key: string) => { - if (key === "__fallback__") { - return router.fallback?.() ?? $.div(); - } - // Find the route that matches this pathname - const matched = findMatch(router, key); - if (Option.isNone(matched)) { - return router.fallback?.() ?? $.div(); - } - return renderRouteWithGuard(matched.value.route, nav, layouts); - }, - })) as HTMLElement | SVGElement; - }) as Element.Element< + // Use pathname as the reconcile key so param-only navigations + // (e.g. /users/alice → /users/bob) trigger a re-render. + return (yield* reconcile(nav.pathname, { + getTargetKeys: (pathname: string) => { + const matched = findMatch(router, pathname); + if (Option.isSome(matched)) return [pathname]; + if (router.fallback) return ["__fallback__"]; + return []; + }, + renderSlot: (key: string) => { + if (key === "__fallback__") { + return router.fallback?.() ?? $.div(); + } + // Find the route that matches this pathname + const matched = findMatch(router, key); + if (Option.isNone(matched)) { + return router.fallback?.() ?? $.div(); + } + return renderRouteWithGuard(matched.value.route, nav, layouts); + }, + })) as HTMLElement | SVGElement; + }), + // Provide the animation config the way `match`/`when` do — reconcile's + // addSlot/removeSlot read `AnimationConfigCtx` lazily to drive enter/ + // exit transitions between routes. Without this, `config.animate` and + // `config.intro` sit in the type but never reach the control ctx. + config.animate || config.intro + ? Effect.provideService(AnimationConfigCtx, { + single: config.animate, + intro: config.intro, + }) + : (x) => x, + ) as Element.Element< HTMLElement | SVGElement, E, R | NavigationContext | ControlCtx