mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
fix(webui): install the Storybook fetch stub on commit and match a Request's own method
Two follow-ups from the CodeRabbit re-review of b8238341.
Install moved out of the render phase. The stub was assigned to `window.fetch`
during render, but a render can be abandoned — interrupted, suspended, or
thrown out — without ever committing, and an abandoned render schedules no
cleanup. That stranded a stub on `window.fetch` with nothing left to remove it.
The install now runs in a layout effect, and `<Story />` is withheld until the
stub is live so a story that fetches on mount still cannot observe the real
`fetch` (the reason the install sat in render to begin with). The layout effect
lands the second render synchronously before paint, so there is no flicker, and
cleanup ordering now falls out naturally: the outgoing story's cleanup runs
before the incoming story's install, with the ownership check retained.
`Request.method` is now honored. `fetch(new Request(url, { method: "POST" }))`
carries its method on the Request rather than on `init`, so reading only `init`
classified it as a GET — matching the wrong route, or none. Resolution order is
`init.method`, then `Request.method`, then `GET`.
Regression tests: an uncommitted render (`renderToStaticMarkup`, which runs the
render phase and stops) must leave `window.fetch` untouched; and a POST
`Request` must select the POST route while a bare `Request` still selects the
GET one. Both verified red against the previous implementation.
Verified: `pnpm test:storybook` 104/104 — withholding the story for one commit
breaks no play function; `pnpm lint` (conventions + tsc); `pnpm test` 1343
passing / +2 tests, with the same 16 pre-existing local failures from the
Node 26 vs required 22.22 happy-dom mismatch (green on CI).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,7 @@ import assert from "node:assert/strict";
|
||||
import type { Decorator } from "@storybook/react-vite";
|
||||
import React, { act } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
import { test, vi } from "vitest";
|
||||
|
||||
import { withStubbedFetch } from "./storybook-decorators";
|
||||
@@ -117,3 +118,62 @@ test("passthrough is opt-in, and only reaches the network for unmatched routes",
|
||||
container.remove();
|
||||
}
|
||||
});
|
||||
|
||||
test("the render phase never installs the stub — only a committed mount does", () => {
|
||||
const realFetch = vi.fn(async () => new Response("REAL BACKEND"));
|
||||
const originalFetch = window.fetch;
|
||||
window.fetch = realFetch as unknown as typeof window.fetch;
|
||||
|
||||
const Alpha = storyComponent(
|
||||
withStubbedFetch([{ match: "/alpha", json: { from: "alpha" } }]),
|
||||
() => <span>alpha</span>,
|
||||
);
|
||||
|
||||
// `renderToStaticMarkup` runs the render phase and stops, which is what an
|
||||
// abandoned render (interrupted, suspended, or thrown out) does. Such a
|
||||
// render schedules no cleanup, so a render-phase install would strand a stub
|
||||
// on `window.fetch` with nothing left to ever remove it.
|
||||
const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
try {
|
||||
renderToStaticMarkup(<Alpha />);
|
||||
assert.equal(window.fetch, realFetch, "an uncommitted render must not touch window.fetch");
|
||||
} finally {
|
||||
consoleError.mockRestore();
|
||||
window.fetch = originalFetch;
|
||||
}
|
||||
});
|
||||
|
||||
test("a Request input is matched on its own method, not treated as GET", async () => {
|
||||
const realFetch = vi.fn(async () => new Response("REAL BACKEND"));
|
||||
const originalFetch = window.fetch;
|
||||
window.fetch = realFetch as unknown as typeof window.fetch;
|
||||
|
||||
const Minting = storyComponent(
|
||||
withStubbedFetch([
|
||||
{ match: "/pairing", json: { from: "status" } },
|
||||
{ match: "/pairing", method: "POST", json: { from: "mint" } },
|
||||
]),
|
||||
() => <span>minting</span>,
|
||||
);
|
||||
|
||||
const container = document.createElement("div");
|
||||
document.body.append(container);
|
||||
const root = createRoot(container);
|
||||
|
||||
try {
|
||||
await act(async () => root.render(<Minting />));
|
||||
|
||||
// The method rides on the Request, not on `init` — reading only `init`
|
||||
// would fall through to the GET route and serve the wrong body.
|
||||
const posted = await window.fetch(new Request("https://host/pairing", { method: "POST" }));
|
||||
assert.deepEqual(await posted.json(), { from: "mint" });
|
||||
|
||||
const got = await window.fetch(new Request("https://host/pairing"));
|
||||
assert.deepEqual(await got.json(), { from: "status" });
|
||||
assert.equal(realFetch.mock.calls.length, 0);
|
||||
} finally {
|
||||
await act(async () => root.unmount());
|
||||
window.fetch = originalFetch;
|
||||
container.remove();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { Decorator } from "@storybook/react-vite";
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { useLayoutEffect, useRef, useState } from "react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { MemoryRouter } from "react-router";
|
||||
|
||||
@@ -79,14 +79,22 @@ type StubbedFetch = typeof window.fetch & {
|
||||
* backend" a property of the harness rather than a convention each new story
|
||||
* has to remember.
|
||||
*
|
||||
* Each decorator instance OWNS its stub. Switching stories renders the incoming
|
||||
* decorator before React runs the outgoing one's cleanup, so a shared
|
||||
* "is a stub already installed?" guard would leave the new story running on the
|
||||
* *old* story's routes and would then let the outgoing cleanup restore the real
|
||||
* `fetch` underneath it — a live backend call from a story. Installing
|
||||
* unconditionally when `window.fetch` is not this instance's own stub, and
|
||||
* restoring only while this instance still owns the active stub, keeps the
|
||||
* handoff hermetic in both directions. The real `fetch` is carried forward via
|
||||
* Each decorator instance OWNS its stub, and installs it from a layout effect
|
||||
* rather than during render. Two ordering hazards drive that shape:
|
||||
*
|
||||
* - A render can be abandoned (interrupted, suspended, or thrown out) without
|
||||
* ever committing, and an abandoned render schedules no cleanup — a
|
||||
* render-phase install would strand a stub on `window.fetch` forever.
|
||||
* - Switching stories mounts the incoming decorator while the outgoing one is
|
||||
* still mounted, so a shared "is a stub installed?" guard would leave the new
|
||||
* story running on the OLD story's routes and let the outgoing cleanup
|
||||
* restore the real `fetch` underneath it — a live backend call from a story.
|
||||
*
|
||||
* `<Story />` is therefore withheld until the stub is live: the story's own
|
||||
* mount effects fetch imperatively, and they must never observe the real
|
||||
* `fetch`. The install runs in a layout effect, so the second render lands
|
||||
* synchronously before paint. Cleanup restores only while this instance still
|
||||
* owns the active stub, and the real `fetch` is carried forward via
|
||||
* `__original` so stubs never chain-wrap each other.
|
||||
*/
|
||||
export function withStubbedFetch(
|
||||
@@ -99,11 +107,9 @@ export function withStubbedFetch(
|
||||
const routesRef = useRef(routes);
|
||||
routesRef.current = routes;
|
||||
const ownedRef = useRef<StubbedFetch | null>(null);
|
||||
const [installed, setInstalled] = useState(false);
|
||||
|
||||
// Install synchronously during render so the story's mount effects already
|
||||
// see the stub. Idempotent under React's double-invoked render: the second
|
||||
// pass sees this instance's own stub already installed.
|
||||
if (typeof window !== "undefined" && window.fetch !== ownedRef.current) {
|
||||
useLayoutEffect(() => {
|
||||
const current = window.fetch as StubbedFetch;
|
||||
const original =
|
||||
current.__storybookStub && current.__original
|
||||
@@ -112,7 +118,11 @@ export function withStubbedFetch(
|
||||
const stub = (async (input: RequestInfo | URL, init?: RequestInit) => {
|
||||
const url =
|
||||
typeof input === "string" ? input : input instanceof URL ? input.href : input.url;
|
||||
const method = (init?.method ?? "GET").toUpperCase();
|
||||
// `fetch(new Request(url, { method: "POST" }))` carries its method on
|
||||
// the Request, not on `init` — reading only `init` would classify it as
|
||||
// a GET and match the wrong route (or none).
|
||||
const requestMethod = input instanceof Request ? input.method : undefined;
|
||||
const method = (init?.method ?? requestMethod ?? "GET").toUpperCase();
|
||||
const route = routesRef.current.find(
|
||||
(r) => (r.method ?? "GET").toUpperCase() === method && url.includes(r.match),
|
||||
);
|
||||
@@ -135,21 +145,21 @@ export function withStubbedFetch(
|
||||
stub.__original = original;
|
||||
ownedRef.current = stub;
|
||||
window.fetch = stub;
|
||||
}
|
||||
useEffect(() => {
|
||||
// Re-assert ownership: React's StrictMode runs cleanup once before the
|
||||
// real mount, and no render follows it to reinstall.
|
||||
const owned = ownedRef.current;
|
||||
if (owned && window.fetch !== owned) window.fetch = owned;
|
||||
setInstalled(true);
|
||||
return () => {
|
||||
const mine = ownedRef.current;
|
||||
// Only the instance that still owns the active stub may restore — an
|
||||
// incoming story that already installed its own must not be torn down.
|
||||
if (mine && window.fetch === mine && mine.__original) {
|
||||
window.fetch = mine.__original;
|
||||
if (window.fetch === stub && stub.__original) {
|
||||
window.fetch = stub.__original;
|
||||
}
|
||||
ownedRef.current = null;
|
||||
setInstalled(false);
|
||||
};
|
||||
}, []);
|
||||
|
||||
// Withheld until the stub is live, so a story that fetches on mount can
|
||||
// never observe the real `fetch`.
|
||||
if (!installed) return null;
|
||||
return <Story />;
|
||||
};
|
||||
}
|
||||
|
||||
@@ -0,0 +1,165 @@
|
||||
import type { Decorator } from "@storybook/react-vite";
|
||||
import { useLayoutEffect, useRef, useState } from "react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { MemoryRouter } from "react-router";
|
||||
|
||||
/**
|
||||
* Shared Storybook decorators for the app-wired `Components/*` stories.
|
||||
*
|
||||
* i18n is NOT provided here: `.storybook/preview.tsx` imports `src/i18n/en` as a
|
||||
* side effect, which populates the default `useT()` pack for every story. These
|
||||
* decorators only supply the router and react-query contexts that the shared
|
||||
* components read from.
|
||||
*/
|
||||
|
||||
/** Wrap a story in a MemoryRouter so `NavLink` / `useNavigate` / `useLocation` work. */
|
||||
export function withRouter(initialPath = "/chat"): Decorator {
|
||||
return (Story) => (
|
||||
<MemoryRouter initialEntries={[initialPath]}>
|
||||
<Story />
|
||||
</MemoryRouter>
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Provide a fresh QueryClient per story mount. `seed` may pre-populate query
|
||||
* data (via `client.setQueryData`) so a component that reads a cached query
|
||||
* renders its loaded state without a network call — `staleTime: Infinity`
|
||||
* prevents a background refetch of the seeded entry.
|
||||
*/
|
||||
export function withQueryClient(seed?: (client: QueryClient) => void): Decorator {
|
||||
return function QueryClientDecorator(Story) {
|
||||
const [client] = useState(() => {
|
||||
const created = new QueryClient({
|
||||
defaultOptions: {
|
||||
queries: { retry: false, staleTime: Infinity, gcTime: Infinity },
|
||||
},
|
||||
});
|
||||
seed?.(created);
|
||||
return created;
|
||||
});
|
||||
return (
|
||||
<QueryClientProvider client={client}>
|
||||
<Story />
|
||||
</QueryClientProvider>
|
||||
);
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* A single stubbed HTTP route for {@link withStubbedFetch}. `match` is a
|
||||
* substring tested against the request URL; `method` defaults to `GET`.
|
||||
* `json` is the response body (a value, or a factory called per request so
|
||||
* time-sensitive fields like `expires_at` stay fresh); omit it for an empty
|
||||
* body. `status` defaults to 200 (or 204 when there is no body).
|
||||
*/
|
||||
export type FetchStubRoute = {
|
||||
match: string;
|
||||
method?: string;
|
||||
status?: number;
|
||||
json?: unknown | (() => unknown);
|
||||
};
|
||||
|
||||
type StubbedFetch = typeof window.fetch & {
|
||||
__storybookStub?: true;
|
||||
/** The real `window.fetch`, carried forward through every stub in a handoff. */
|
||||
__original?: typeof window.fetch;
|
||||
};
|
||||
|
||||
/**
|
||||
* Stub `window.fetch` for the story lifetime so components that fetch
|
||||
* imperatively on mount (not through a react-query cache `withQueryClient` can
|
||||
* seed) receive deterministic responses instead of hitting a real backend — a
|
||||
* shared/deployed Storybook must never perform a real side effect (e.g. minting
|
||||
* a live pairing code). Matched routes return a JSON `Response`; anything
|
||||
* unmatched REJECTS by default, naming the URL — a story that forgets a route
|
||||
* fails loudly instead of quietly acquiring live network access. Pass
|
||||
* `{ passthrough: true }` for the rare story that genuinely needs the real
|
||||
* `fetch`; making that opt-in is what keeps "shared Storybook never reaches a
|
||||
* backend" a property of the harness rather than a convention each new story
|
||||
* has to remember.
|
||||
*
|
||||
* Each decorator instance OWNS its stub, and installs it from a layout effect
|
||||
* rather than during render. Two ordering hazards drive that shape:
|
||||
*
|
||||
* - A render can be abandoned (interrupted, suspended, or thrown out) without
|
||||
* ever committing, and an abandoned render schedules no cleanup — a
|
||||
* render-phase install would strand a stub on `window.fetch` forever.
|
||||
* - Switching stories mounts the incoming decorator while the outgoing one is
|
||||
* still mounted, so a shared "is a stub installed?" guard would leave the new
|
||||
* story running on the OLD story's routes and let the outgoing cleanup
|
||||
* restore the real `fetch` underneath it — a live backend call from a story.
|
||||
*
|
||||
* `<Story />` is therefore withheld until the stub is live: the story's own
|
||||
* mount effects fetch imperatively, and they must never observe the real
|
||||
* `fetch`. The install runs in a layout effect, so the second render lands
|
||||
* synchronously before paint. Cleanup restores only while this instance still
|
||||
* owns the active stub, and the real `fetch` is carried forward via
|
||||
* `__original` so stubs never chain-wrap each other.
|
||||
*/
|
||||
export function withStubbedFetch(
|
||||
routes: FetchStubRoute[],
|
||||
options: { passthrough?: boolean } = {},
|
||||
): Decorator {
|
||||
return function StubbedFetchDecorator(Story) {
|
||||
// Read through a ref so the installed stub always serves this render's
|
||||
// routes, even if the same instance is re-rendered with new ones.
|
||||
const routesRef = useRef(routes);
|
||||
routesRef.current = routes;
|
||||
const ownedRef = useRef<StubbedFetch | null>(null);
|
||||
const [installed, setInstalled] = useState(false);
|
||||
|
||||
useLayoutEffect(() => {
|
||||
const current = window.fetch as StubbedFetch;
|
||||
const original =
|
||||
current.__storybookStub && current.__original
|
||||
? current.__original
|
||||
: window.fetch.bind(window);
|
||||
const stub = (async (input: RequestInfo | URL, init?: RequestInit) => {
|
||||
const url =
|
||||
typeof input === "string" ? input : input instanceof URL ? input.href : input.url;
|
||||
// `fetch(new Request(url, { method: "POST" }))` carries its method on
|
||||
// the Request, not on `init` — reading only `init` would classify it as
|
||||
// a GET and match the wrong route (or none).
|
||||
const requestMethod = input instanceof Request ? input.method : undefined;
|
||||
const method = (init?.method ?? requestMethod ?? "GET").toUpperCase();
|
||||
const route = routesRef.current.find(
|
||||
(r) => (r.method ?? "GET").toUpperCase() === method && url.includes(r.match),
|
||||
);
|
||||
if (!route) {
|
||||
if (options.passthrough) return original(input, init);
|
||||
throw new Error(
|
||||
`withStubbedFetch: unmatched ${method} ${url}. Stories must not reach a real ` +
|
||||
"backend — add a route for it, or pass { passthrough: true } if this story " +
|
||||
"genuinely needs the network.",
|
||||
);
|
||||
}
|
||||
const value = typeof route.json === "function" ? (route.json as () => unknown)() : route.json;
|
||||
const hasBody = value !== undefined;
|
||||
return new Response(hasBody ? JSON.stringify(value) : null, {
|
||||
status: route.status ?? (hasBody ? 200 : 204),
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}) as StubbedFetch;
|
||||
stub.__storybookStub = true;
|
||||
stub.__original = original;
|
||||
ownedRef.current = stub;
|
||||
window.fetch = stub;
|
||||
setInstalled(true);
|
||||
return () => {
|
||||
// Only the instance that still owns the active stub may restore — an
|
||||
// incoming story that already installed its own must not be torn down.
|
||||
if (window.fetch === stub && stub.__original) {
|
||||
window.fetch = stub.__original;
|
||||
}
|
||||
ownedRef.current = null;
|
||||
setInstalled(false);
|
||||
};
|
||||
}, []);
|
||||
|
||||
// Withheld until the stub is live, so a story that fetches on mount can
|
||||
// never observe the real `fetch`.
|
||||
if (!installed) return null;
|
||||
return <Story />;
|
||||
};
|
||||
}
|
||||
Reference in New Issue
Block a user