diff --git a/docs/scoping.md b/docs/scoping.md index 0ad519c..c1da5b8 100644 --- a/docs/scoping.md +++ b/docs/scoping.md @@ -80,8 +80,8 @@ export default { }, { left: "/docs", beforeShot: async (page) => { await page.click("[data-tab=intro]"); } }, ], - // For every route without its own `before` (which replaces it), CLI routes included; - // a top-level `beforeShot` works the same way. + // For every route without its own `before` (which replaces it; that one can still run it + // with `await topLevel?.()`), CLI routes included; a top-level `beforeShot` works the same way. before: async (page, { side, route, width }) => { const accept = page.getByRole("button", { name: "Accept" }); if (await accept.isVisible()) await accept.click(); diff --git a/renderdiff.config.example.ts b/renderdiff.config.example.ts index 31df8a0..97e63fd 100644 --- a/renderdiff.config.example.ts +++ b/renderdiff.config.example.ts @@ -24,10 +24,12 @@ export default { { left: "/blog", right: "/posts" }, { left: "/pricing", selector: { left: ".plans-v2", right: ".plans" } }, // `before` runs on each side after the page loads, before it settles and is shot, so - // the open menu is what gets measured. Replaces the top-level `before` for this route. + // the open menu is what gets measured. Replaces the top-level `before` for this route, + // which `topLevel` still runs. { left: "/docs", - before: async (page) => { + before: async (page, { topLevel }) => { + await topLevel?.(); await page.click("[aria-label=Menu]"); }, }, diff --git a/skills/renderdiff/reference.md b/skills/renderdiff/reference.md index 9170d92..7640bee 100644 --- a/skills/renderdiff/reference.md +++ b/skills/renderdiff/reference.md @@ -298,7 +298,8 @@ aria: identical on 2 of 3 route(s), differs /pricing transformed element is shot at its axis-aligned box. `xpath=` and `>>` work; pseudo-elements are rejected; iframes can't be reached. - **`before` hooks** (config only: `before: async (page, { side, route, width }) => …` on a - route object, or top level for every route without its own) run after load and the HTTP + route object, or top level for every route without its own; a route's own gets `topLevel` + too, which runs the top-level one: `await topLevel?.()`) run after load and the HTTP check, before settle, masks and the shot. Legitimate: open a menu or tab, dismiss a modal or banner *both* sides show, sign in, set a carousel to slide 1 on both. Not: branching on `side` to make the sides look alike, hiding or removing an element on one side. `side` is for diff --git a/src/cache.ts b/src/cache.ts index 0755399..40942c0 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -15,6 +15,8 @@ export const shotId = (route: Route, width: number) => width, route.before && String(route.before), route.beforeShot && String(route.beforeShot), + route.topLevel.before && String(route.topLevel.before), + route.topLevel.beforeShot && String(route.topLevel.beforeShot), ]); /** Everything that changes what the right origin serves, so one recording never answers another. */ diff --git a/src/capture.ts b/src/capture.ts index b2a38c1..2076f76 100644 --- a/src/capture.ts +++ b/src/capture.ts @@ -286,7 +286,9 @@ export async function capture( if (!fn) return; step.set(page, `${name} hook`); try { - await fn(page, { side, route: label(route), width }); + const ctx = { side, route: label(route), width }; + const top = route.topLevel[name]; + await fn(page, top ? { ...ctx, topLevel: () => top(page, ctx) } : ctx); } catch (e) { throw new HookError( `${name} hook failed on ${side}: ${e instanceof Error ? e.message : String(e)}`, diff --git a/src/config.ts b/src/config.ts index f9eb962..3b8f313 100644 --- a/src/config.ts +++ b/src/config.ts @@ -25,16 +25,19 @@ export interface Route { selector: Sides | null; before: Hook | null; beforeShot: Hook | null; + /** The top-level hooks this route's own hooks replace, for their `ctx.topLevel`. */ + topLevel: { before: Hook | null; beforeShot: Hook | null }; } /** * Runs on each side, e.g. to open a menu: `before` after the page loads, before it settles; * `beforeShot` after it settles, right before masks and the shot. `route` is the route's name - * (`/a`, or `/a::/b` for a pair), the same on both sides. + * (`/a`, or `/a::/b` for a pair), the same on both sides. `topLevel`, on a route's own hook, + * runs the top-level hook it replaces, when there is one. */ export type Hook = ( page: Page, - ctx: { side: "left" | "right"; route: string; width: number }, + ctx: { side: "left" | "right"; route: string; width: number; topLevel?: () => Promise }, ) => Promise; /** @@ -68,7 +71,7 @@ export const name = (r: Pick) => * literal), which keeps `?q=std::vector` and `/wiki/Talk:Foo` single routes. Each side may end * in whitespace + `@sel` (the whitespace keeps `/@user` a path), which beats a CLI `selector`, * which beats a config object's own. A route's own `before` (or `beforeShot`) replaces the - * top-level one. + * top-level one, which it can still call as `ctx.topLevel`. */ export function parseRoutes( specs: RouteSpec[], @@ -106,20 +109,26 @@ export function parseRoutes( const glob = sides.find((s) => s.includes("*")); if (glob) throw new Error(`globs are not supported (${glob}) — pass literal paths.`); const own = typeof spec === "string" ? undefined : spec.selector; + const hooks: { before?: Hook; beforeShot?: Hook } = typeof spec === "string" ? {} : spec; + const top = (h: Hook | undefined, t: Hook | undefined) => (h && t && h !== t ? t : null); const route = { left: normalize(sides[0]), right: normalize(sides[1]), selector: atSel ?? selector ?? (typeof own === "string" ? { left: own, right: own } : (own ?? null)), - before: (typeof spec === "string" ? undefined : spec.before) ?? before ?? null, - beforeShot: (typeof spec === "string" ? undefined : spec.beforeShot) ?? beforeShot ?? null, + before: hooks.before ?? before ?? null, + beforeShot: hooks.beforeShot ?? beforeShot ?? null, + topLevel: { + before: top(hooks.before, before), + beforeShot: top(hooks.beforeShot, beforeShot), + }, }; const same = out.get(name(route)); // JSON drops a function but keeps a null, so hooks are compared on their own below. if ( same && - JSON.stringify({ ...same, before: 0, beforeShot: 0 }) !== - JSON.stringify({ ...route, before: 0, beforeShot: 0 }) + JSON.stringify({ ...same, before: 0, beforeShot: 0, topLevel: 0 }) !== + JSON.stringify({ ...route, before: 0, beforeShot: 0, topLevel: 0 }) ) { throw new Error(`two routes both read as ${name(route)}; change one of their selectors`); } @@ -277,9 +286,15 @@ export interface RenderdiffConfig { right?: string; /** Routes to diff when none are passed on the command line. A route object may take a `selector`, `before` and `beforeShot`. */ routes?: RouteSpec[]; - /** A {@link Hook} for every route without its own `before`, CLI routes included. */ + /** + * A {@link Hook} for every route without its own `before` (which can call it as + * `ctx.topLevel`), CLI routes included. + */ before?: Hook; - /** A {@link Hook} for every route without its own `beforeShot`, CLI routes included. */ + /** + * A {@link Hook} for every route without its own `beforeShot` (which can call it as + * `ctx.topLevel`), CLI routes included. + */ beforeShot?: Hook; /** * Playwright storage state files (cookies, localStorage) to start each side with, e.g. to be diff --git a/tests/unit/cache.test.ts b/tests/unit/cache.test.ts index f63d79f..0bb913f 100644 --- a/tests/unit/cache.test.ts +++ b/tests/unit/cache.test.ts @@ -7,7 +7,7 @@ import fs from "node:fs"; import path from "node:path"; import { after, describe, test } from "node:test"; import { cacheFile, replayable, REPLAY_TTL, saveRecording, shotId } from "../../src/cache.ts"; -import { parseRoutes, resolveConfig } from "../../src/config.ts"; +import { type Hook, parseRoutes, resolveConfig } from "../../src/config.ts"; import { cleanupTempDirs, tempDir } from "../helpers/cli.ts"; after(cleanupTempDirs); @@ -106,6 +106,17 @@ describe("keys", () => { assert.notEqual(shotId(route!, 1440), shotId(hooked, 1440)); const shotHooked = parseRoutes(["/a"], undefined, undefined, async () => {})[0]!; assert.notEqual(shotId(route!, 1440), shotId(shotHooked, 1440)); + const own = async () => {}; + const withTop = (top: Hook) => + shotId(parseRoutes([{ left: "/a", before: own }], undefined, top)[0]!, 1440); + const reload: Hook = async (page) => { + await page.reload(); + }; + assert.notEqual( + withTop(async () => {}), + withTop(reload), + "the top-level hook it calls", + ); assert.notEqual(shotId(route!, 1440), shotId(route!, 375)); assert.equal(shotId(route!, 1440), shotId(paired, 1440), "the left path doesn't matter"); }); diff --git a/tests/visual/guardrails.test.ts b/tests/visual/guardrails.test.ts index ec5ff90..6a0c450 100644 --- a/tests/visual/guardrails.test.ts +++ b/tests/visual/guardrails.test.ts @@ -910,6 +910,16 @@ describe("before hooks", () => { assert.equal(result(results(run.outDir), "/menu", 1440).diffPixels, 0, context(run)); }); + test("a route's own hook can run the top-level one as ctx.topLevel", async () => { + const run = await hooked( + '{ before: async (page, { side }) => { await page.locator("h1").evaluate((h, t) => (h.textContent = t), side); }, ' + + 'routes: [{ left: "/menu", before: async (page, { topLevel }) => { await topLevel?.(); } }] }', + ); + assert.equal(run.code, 1, context(run)); + const r = result(results(run.outDir), "/menu", 1440); + assert.deepEqual([r.text.added, r.text.removed], [["left"], ["right"]], context(run)); + }); + test("a hook that throws is a route error naming the hook, and is not retried", async () => { const before = site.hits("/menu"); // A message the transient-error retry would otherwise match.