mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
cda19e1c58
* fix(app-router): preserve recent segment state with Activity BFCache Fresh App Router navigations were reusing the active React tree when moving between dynamic segment values. That leaked client state into a newly entered segment and made the upstream back-forward-cache suite fail before it even reached browser traversal assertions. The root cause was that vinext had history-entry bfcacheId plumbing but no separate segment state-key cache. Next.js treats those identities separately: render state is keyed by segment identity/pathname, while useRouter().bfcacheId identifies a concrete history entry. Add state-key maps for app router slots, render retained entries through React.Activity when cacheComponents is enabled, keep the active one-entry keyed reset path otherwise, and port focused Next.js regression coverage for the three-entry BFCache behavior. * fix(bfcache): store stateKeyMap in retained slot entries and re-provide on render Review feedback requested two changes: 1. Add stateKeyMap to BfcacheSlotEntry so each retained BFCache entry carries the state-key map that belonged to it when active, rather than borrowing the current navigation's map. This prevents nested hidden entries from being rendered with old elements but new pathname-derived state keys — an identity corruption bug that upstream Next.js avoids by rendering each retained entry from its own state. 2. Add a nested dynamic regression test: /nested/[section]/item/[id] with client counters in both the [section] layout and [id] page. The existing flat /page/[n] tests do not exercise nested retained-entry identity, which is exactly where the missing state-key-map bug lives. Changes: - BfcacheSlotEntry gains stateKeyMap?: Readonly<Record<string, string>> - updateBfcacheSlotEntries accepts and stores an optional stateKeyMap argument - BfcacheSlotBoundary stores the current stateKeyMap alongside elements when caching the active entry - Both the Activity visible/hidden path and the non-cacheComponents single-entry path now wrap rendered entries in BfcacheStateKeyMapContext.Provider using the entry's own stateKeyMap (falling back to current map for forward compatibility) - New e2e test verifies nested layout + page counter preservation across segment navigations * refactor(bfcache): extract shared provider stack into BfcacheEntryProviders BfcacheSlotBoundary rendered the BfcacheStateKeyMapContext / ElementsContext / SegmentContext provider stack in two places: the one-entry keyed reset path and the per-entry Activity retention list. The two stacks were identical except for where the React key sat, so the retained-entry bookkeeping and its render shape were tangled together and harder to audit than needed. Extract BfcacheEntryProviders as a pure presentational helper and render it from both paths. The keyed-reset path keeps its remount semantics: the key now sits on the BfcacheEntryProviders element, so a fresh active stateKey still remounts the whole provider subtree. Hook ordering is unchanged. All hooks stay above the early returns in BfcacheSlotBoundary, so the boundary still satisfies the Rules of Hooks across the activeStateKey undefined-to-defined transition. Covered by the slot BFCache entry-state suite in tests/shims.test.ts. * test(bfcache): isolate cacheComponents BFCache coverage in a dedicated fixture The back/forward-cache e2e suite ran against the shared app-basic fixture with cacheComponents: true. app-basic backs the single app-router Playwright server that every app-router spec uses, so enabling cacheComponents globally changed a core DOM invariant for unrelated tests: inactive route trees stay mounted as hidden Activity DOM. The contamination already leaked a :visible workaround into the interception-modal bfcacheId spec, which had nothing to do with Activity. cacheComponents is an opt-in semantic mode, not default app-router behaviour, so it does not belong on the shared baseline fixture. Move the BFCache routes into a dedicated tests/fixtures/app-bfcache fixture with cacheComponents: true, served by a new app-router-bfcache Playwright project on port 4183, and run the back-forward-cache spec there against the project baseURL. The :visible locators stay in that spec, where duplicate hidden DOM is the expected contract. Remove cacheComponents and the BFCache routes from app-basic so the shared fixture models default behaviour again, and revert the :visible workaround in use-router-bfcache-id.spec.ts now that the modal route no longer retains hidden DOM. Fix the nested item page import while moving it: nested/[section]/item/[id]/ page.tsx imported ../counter, but the counter lives two levels up at nested/[section]/counter.tsx, so it must be ../../counter. Validation: PLAYWRIGHT_PROJECT=app-router-bfcache (5 passed) and the full use-router-bfcache-id spec under app-router (11 passed), plus the slot BFCache unit suite (1172 passed) and vp check (formatting + lint + types clean). * ci: run the app-router-bfcache e2e project in the matrix The dedicated app-router-bfcache Playwright project was added to playwright.config.ts but not to the CI E2E matrix, so the back/forward-cache specs never ran in CI. Add it to the matrix project list so the BFCache suite runs on every push. It uses the default chromium browser, so the existing setup and browser-cache steps apply unchanged. * fix(bfcache): keep persisted slots mounted when cacheComponents is off Without cacheComponents there is no Activity retention, yet BfcacheSlotBoundary still keyed the active entry by its segment stateKey. That stateKey tracks the pathname (createBfcacheSegmentIdentity), so any slot whose identity moves with the URL (shared layouts, interception source slots) was remounted on navigation, discarding client state that survives a normal route change. The baseline router reconciles these slots in place. This regressed seven existing app-router e2e tests once cacheComponents was no longer masking it on the shared fixture: interception source-context and modal preservation in advanced.spec.ts, and layout-persistence.spec.ts dynamic-segment layout counter survival. Fresh-entry reset is driven by userland bfcacheId keying in the fixtures, not by remounting the slot subtree, so the keying was pure overreach in the non-cacheComponents path. Render the single active entry unkeyed when cacheComponents is off so the boundary reconciles in place like the baseline. The keyed Activity retention path is unchanged for cacheComponents. Validation: full app-router project 436 passed / 0 failed (the seven regressions now pass); use-router-bfcache-id 11 passed (fresh-push reset intact); app-bfcache 5 passed (Activity retention intact); shims unit suite 1024 passed. * docs(test): point bfcacheId spec comment at the app-router-bfcache project The header noted that Activity-backed preservation is covered in back-forward-cache.spec.ts, but that spec moved to the dedicated app-bfcache fixture under the app-router-bfcache project. Name the project and path so the cross-reference stays accurate. * docs: align cacheComponents comment with unkeyed active-segment behavior * test(bfcache): assert fresh bfcacheId after Activity-restored return * style: collapse locator chains per oxfmt * refactor(bfcache): split slot entry order from snapshots * fix(bfcache): keep non-cacheComponents slot path baseline * fix(bfcache): use string cacheComponents define * test: remove external fetch from revalidate tag fixture * fix(bfcache): harden Activity state key retention * docs(bfcache): document cacheComponents Activity caveats * fix(bfcache): preserve navigation context for fallback SSR Fallback boundary rendering can outlive the original request-scoped navigation context. That made app SSR fail fast with a missing navigation context when loading-boundary digest routes rendered their HTTP fallback shell. The SSR boundary contract still requires a navigation context for real request renders, but boundary renderers now synthesize the same typed shape from the request URL and matched params when request scope has already been cleared. * refactor(bfcache): normalize pathname in state-key map, document invariant callers Moves hydration-pathname normalization into createBfcacheSegmentStateKeyMap so SSR and client always produce byte-identical Activity keys. Documents the requireNavigationContext guaranteeing callers per review feedback. * fix(bfcache): tolerate malformed state-key pathnames * docs(bfcache): document known Next.js divergence and dead-branch contract * chore(deps): update lockfile for vite 0.1.24 * fix(app-router): preserve encoded delimiters in bfcache keys * docs(bfcache): track router-owned retention follow-up * chore: retrigger ci * fix(app-router): preserve redirect bridge fallback
424 lines
14 KiB
TypeScript
424 lines
14 KiB
TypeScript
import { describe, expect, it, vi } from "vite-plus/test";
|
|
import {
|
|
createAppPageFontData,
|
|
createAppPageRscErrorTracker,
|
|
deferUntilStreamConsumed,
|
|
renderAppPageHtmlResponse,
|
|
renderAppPageHtmlStream,
|
|
renderAppPageHtmlStreamWithRecovery,
|
|
shouldRerenderAppPageWithGlobalError,
|
|
} from "../packages/vinext/src/server/app-page-stream.js";
|
|
|
|
function createStream(chunks: string[]): ReadableStream<Uint8Array> {
|
|
return new ReadableStream({
|
|
start(controller) {
|
|
for (const chunk of chunks) {
|
|
controller.enqueue(new TextEncoder().encode(chunk));
|
|
}
|
|
controller.close();
|
|
},
|
|
});
|
|
}
|
|
|
|
describe("app page stream helpers", () => {
|
|
it("collects app page font data from RSC environment getters", () => {
|
|
expect(
|
|
createAppPageFontData({
|
|
getLinks() {
|
|
return ["/font.css"];
|
|
},
|
|
getPreloads() {
|
|
return [{ href: "/font.woff2", type: "font/woff2" }];
|
|
},
|
|
getStyles() {
|
|
return [".font { font-family: Test; }"];
|
|
},
|
|
}),
|
|
).toEqual({
|
|
links: ["/font.css"],
|
|
preloads: [{ href: "/font.woff2", type: "font/woff2" }],
|
|
styles: [".font { font-family: Test; }"],
|
|
});
|
|
});
|
|
|
|
it("renders the HTML stream through the SSR handler", async () => {
|
|
const fontData = createAppPageFontData({
|
|
getLinks: () => ["/font.css"],
|
|
getPreloads: () => [{ href: "/font.woff2", type: "font/woff2" }],
|
|
getStyles: () => [],
|
|
});
|
|
|
|
const { htmlStream } = await renderAppPageHtmlStream({
|
|
fontData,
|
|
navigationContext: { pathname: "/test", searchParams: new URLSearchParams(), params: {} },
|
|
rscStream: createStream(["flight"]),
|
|
ssrHandler: {
|
|
async handleSsr(_rscStream, navigationContext, receivedFontData) {
|
|
expect(navigationContext).toEqual({
|
|
pathname: "/test",
|
|
searchParams: new URLSearchParams(),
|
|
params: {},
|
|
});
|
|
expect(receivedFontData).toEqual(fontData);
|
|
return createStream(["<html>ok</html>"]);
|
|
},
|
|
},
|
|
});
|
|
|
|
await expect(new Response(htmlStream).text()).resolves.toBe("<html>ok</html>");
|
|
});
|
|
|
|
it("forwards waitForAllReady to the SSR handler", async () => {
|
|
const ssrHandler = vi.fn(async () => createStream(["<html>all-ready</html>"]));
|
|
|
|
const { htmlStream } = await renderAppPageHtmlStream({
|
|
fontData: createAppPageFontData({
|
|
getLinks: () => [],
|
|
getPreloads: () => [],
|
|
getStyles: () => [],
|
|
}),
|
|
navigationContext: null,
|
|
rscStream: createStream(["flight"]),
|
|
waitForAllReady: true,
|
|
ssrHandler: { handleSsr: ssrHandler },
|
|
});
|
|
|
|
await expect(new Response(htmlStream).text()).resolves.toBe("<html>all-ready</html>");
|
|
expect(ssrHandler).toHaveBeenCalledTimes(1);
|
|
expect(ssrHandler).toHaveBeenCalledWith(
|
|
expect.anything(),
|
|
null,
|
|
expect.anything(),
|
|
expect.objectContaining({ waitForAllReady: true }),
|
|
);
|
|
});
|
|
|
|
it("forwards form state to the SSR handler", async () => {
|
|
const formState = ["action-result", "key-path", "reference-id", 1] as never;
|
|
const ssrHandler = vi.fn(async () => createStream(["<html>form-state</html>"]));
|
|
|
|
const { htmlStream } = await renderAppPageHtmlStream({
|
|
fontData: createAppPageFontData({
|
|
getLinks: () => [],
|
|
getPreloads: () => [],
|
|
getStyles: () => [],
|
|
}),
|
|
formState,
|
|
navigationContext: null,
|
|
rscStream: createStream(["flight"]),
|
|
ssrHandler: { handleSsr: ssrHandler },
|
|
});
|
|
|
|
await expect(new Response(htmlStream).text()).resolves.toBe("<html>form-state</html>");
|
|
expect(ssrHandler).toHaveBeenCalledWith(
|
|
expect.anything(),
|
|
null,
|
|
expect.anything(),
|
|
expect.objectContaining({ formState }),
|
|
);
|
|
});
|
|
|
|
it("forwards basePath to the SSR handler", async () => {
|
|
const ssrHandler = vi.fn(async () => createStream(["<html>base-path</html>"]));
|
|
|
|
const { htmlStream } = await renderAppPageHtmlStream({
|
|
basePath: "/docs",
|
|
fontData: createAppPageFontData({
|
|
getLinks: () => [],
|
|
getPreloads: () => [],
|
|
getStyles: () => [],
|
|
}),
|
|
navigationContext: null,
|
|
rscStream: createStream(["flight"]),
|
|
ssrHandler: { handleSsr: ssrHandler },
|
|
});
|
|
|
|
await expect(new Response(htmlStream).text()).resolves.toBe("<html>base-path</html>");
|
|
expect(ssrHandler).toHaveBeenCalledWith(
|
|
expect.anything(),
|
|
null,
|
|
expect.anything(),
|
|
expect.objectContaining({ basePath: "/docs" }),
|
|
);
|
|
});
|
|
|
|
it("defers clearRequestContext until the HTML stream body is fully consumed", async () => {
|
|
// Regression test for issue #660: clearRequestContext() must not race the
|
|
// lazy RSC/SSR stream pipeline. It should be called only after the HTTP
|
|
// response body has been fully consumed by the downstream consumer.
|
|
const contextCleared: string[] = [];
|
|
const clearRequestContext = vi.fn(() => {
|
|
contextCleared.push("cleared");
|
|
});
|
|
|
|
const response = await renderAppPageHtmlResponse({
|
|
clearRequestContext,
|
|
fontData: {
|
|
links: [],
|
|
preloads: [],
|
|
styles: [],
|
|
},
|
|
navigationContext: null,
|
|
rscStream: createStream(["flight"]),
|
|
ssrHandler: {
|
|
async handleSsr() {
|
|
return createStream(["<html>page</html>"]);
|
|
},
|
|
},
|
|
status: 200,
|
|
});
|
|
|
|
// The context must NOT be cleared yet — the response stream hasn't been
|
|
// consumed by the downstream caller (i.e. the HTTP layer) yet.
|
|
expect(contextCleared).toHaveLength(0);
|
|
|
|
// Consuming the stream simulates the HTTP layer reading the response body.
|
|
await response.text();
|
|
|
|
// Now that the stream is fully consumed, context must have been cleared.
|
|
expect(contextCleared).toHaveLength(1);
|
|
});
|
|
|
|
it("calls onFlush when the upstream stream errors mid-consumption", async () => {
|
|
const onFlush = vi.fn();
|
|
const streamError = new Error("component threw during streaming");
|
|
|
|
// Emit one chunk, then error on the next pull — simulates a component
|
|
// throwing partway through RSC/SSR streaming.
|
|
let pullCount = 0;
|
|
const source = new ReadableStream<Uint8Array>({
|
|
pull(controller) {
|
|
pullCount++;
|
|
if (pullCount === 1) {
|
|
controller.enqueue(new TextEncoder().encode("partial"));
|
|
} else {
|
|
controller.error(streamError);
|
|
}
|
|
},
|
|
});
|
|
|
|
const wrapped = deferUntilStreamConsumed(source, onFlush);
|
|
const reader = wrapped.getReader();
|
|
|
|
// First read succeeds with the enqueued chunk.
|
|
const { value } = await reader.read();
|
|
expect(new TextDecoder().decode(value)).toBe("partial");
|
|
|
|
// Second read should surface the upstream error.
|
|
await expect(reader.read()).rejects.toThrow("component threw during streaming");
|
|
|
|
// onFlush must have been called despite the error — this is the bug fix.
|
|
expect(onFlush).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("calls onFlush only once when the stream errors then is cancelled", async () => {
|
|
const onFlush = vi.fn();
|
|
const streamError = new Error("stream error");
|
|
|
|
// Error on the very first pull — simulates immediate failure.
|
|
const source = new ReadableStream<Uint8Array>({
|
|
pull(controller) {
|
|
controller.error(streamError);
|
|
},
|
|
});
|
|
|
|
const wrapped = deferUntilStreamConsumed(source, onFlush);
|
|
const reader = wrapped.getReader();
|
|
|
|
// Reading the errored stream triggers the error handler.
|
|
await expect(reader.read()).rejects.toThrow("stream error");
|
|
|
|
// The idempotent once() guard prevents double invocation — even if
|
|
// some code path triggered cleanup again, onFlush fires exactly once.
|
|
expect(onFlush).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("builds an HTML response, including link headers, and defers clearing request context until after body is consumed", async () => {
|
|
const clearRequestContext = vi.fn();
|
|
|
|
const response = await renderAppPageHtmlResponse({
|
|
clearRequestContext,
|
|
fontData: {
|
|
links: [],
|
|
preloads: [],
|
|
styles: [],
|
|
},
|
|
fontLinkHeader: "</font.woff2>; rel=preload; as=font; type=font/woff2; crossorigin",
|
|
navigationContext: null,
|
|
rscStream: createStream(["flight"]),
|
|
ssrHandler: {
|
|
async handleSsr() {
|
|
return createStream(["<html>page</html>"]);
|
|
},
|
|
},
|
|
status: 203,
|
|
});
|
|
|
|
// Context must NOT be cleared before body is consumed (see issue #660).
|
|
expect(clearRequestContext).toHaveBeenCalledTimes(0);
|
|
expect(response.status).toBe(203);
|
|
expect(response.headers.get("content-type")).toBe("text/html; charset=utf-8");
|
|
expect(response.headers.get("link")).toBe(
|
|
"</font.woff2>; rel=preload; as=font; type=font/woff2; crossorigin",
|
|
);
|
|
await expect(response.text()).resolves.toBe("<html>page</html>");
|
|
|
|
// After body is consumed, context must be cleared exactly once.
|
|
expect(clearRequestContext).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("returns the HTML stream and marks shell render completion when SSR succeeds", async () => {
|
|
const onShellRendered = vi.fn();
|
|
|
|
const result = await renderAppPageHtmlStreamWithRecovery({
|
|
onShellRendered,
|
|
async renderErrorBoundaryResponse() {
|
|
throw new Error("should not render an error boundary");
|
|
},
|
|
async renderHtmlStream() {
|
|
return createStream(["<html>ok</html>"]);
|
|
},
|
|
async renderSpecialErrorResponse() {
|
|
throw new Error("should not render a special response");
|
|
},
|
|
resolveSpecialError() {
|
|
return null;
|
|
},
|
|
});
|
|
|
|
expect(onShellRendered).toHaveBeenCalledTimes(1);
|
|
expect(result.response).toBeNull();
|
|
await expect(new Response(result.htmlStream).text()).resolves.toBe("<html>ok</html>");
|
|
});
|
|
|
|
it("turns special SSR failures into the provided response", async () => {
|
|
const ssrError = new Error("redirect");
|
|
const renderSpecialErrorResponse = vi.fn(async () => new Response("special", { status: 307 }));
|
|
|
|
const result = await renderAppPageHtmlStreamWithRecovery({
|
|
async renderErrorBoundaryResponse() {
|
|
throw new Error("should not render an error boundary");
|
|
},
|
|
async renderHtmlStream() {
|
|
throw ssrError;
|
|
},
|
|
renderSpecialErrorResponse,
|
|
resolveSpecialError(error) {
|
|
return error === ssrError
|
|
? {
|
|
kind: "redirect",
|
|
location: "/target",
|
|
statusCode: 307,
|
|
}
|
|
: null;
|
|
},
|
|
});
|
|
|
|
expect(renderSpecialErrorResponse).toHaveBeenCalledWith({
|
|
kind: "redirect",
|
|
location: "/target",
|
|
statusCode: 307,
|
|
});
|
|
expect(result.htmlStream).toBeNull();
|
|
expect(result.response?.status).toBe(307);
|
|
await expect(result.response?.text()).resolves.toBe("special");
|
|
});
|
|
|
|
it("falls back to the error boundary response for non-special SSR failures", async () => {
|
|
const ssrError = new Error("boom");
|
|
const renderErrorBoundaryResponse = vi.fn(
|
|
async () => new Response("boundary", { status: 200 }),
|
|
);
|
|
|
|
const result = await renderAppPageHtmlStreamWithRecovery({
|
|
renderErrorBoundaryResponse,
|
|
async renderHtmlStream() {
|
|
throw ssrError;
|
|
},
|
|
async renderSpecialErrorResponse() {
|
|
throw new Error("should not render a special response");
|
|
},
|
|
resolveSpecialError() {
|
|
return null;
|
|
},
|
|
});
|
|
|
|
expect(renderErrorBoundaryResponse).toHaveBeenCalledWith(ssrError);
|
|
expect(result.htmlStream).toBeNull();
|
|
expect(result.response?.status).toBe(200);
|
|
await expect(result.response?.text()).resolves.toBe("boundary");
|
|
});
|
|
|
|
it("tracks non-navigation RSC errors while preserving the base onError callback", () => {
|
|
const baseOnError = vi.fn(() => "base-result");
|
|
const tracker = createAppPageRscErrorTracker(baseOnError);
|
|
|
|
expect(tracker.onRenderError(new Error("boom"), { path: "/test" }, { chunk: 1 })).toBe(
|
|
"base-result",
|
|
);
|
|
expect(tracker.getCapturedError()).toBeInstanceOf(Error);
|
|
|
|
tracker.onRenderError({ digest: "NEXT_NOT_FOUND" }, { path: "/test" }, { chunk: 2 });
|
|
expect((tracker.getCapturedError() as Error).message).toBe("boom");
|
|
expect(baseOnError).toHaveBeenCalledTimes(2);
|
|
});
|
|
|
|
it("only rerenders with global-error when an RSC error was captured and no local boundary exists", () => {
|
|
expect(
|
|
shouldRerenderAppPageWithGlobalError({
|
|
capturedError: new Error("boom"),
|
|
hasLocalBoundary: false,
|
|
}),
|
|
).toBe(true);
|
|
|
|
expect(
|
|
shouldRerenderAppPageWithGlobalError({
|
|
capturedError: new Error("boom"),
|
|
hasLocalBoundary: true,
|
|
}),
|
|
).toBe(false);
|
|
|
|
expect(
|
|
shouldRerenderAppPageWithGlobalError({
|
|
capturedError: null,
|
|
hasLocalBoundary: false,
|
|
}),
|
|
).toBe(false);
|
|
});
|
|
|
|
it("emits the `x-edge-runtime: 1` marker on HTML stream responses for edge-runtime routes", async () => {
|
|
const response = await renderAppPageHtmlResponse({
|
|
clearRequestContext: vi.fn(),
|
|
fontData: { links: [], preloads: [], styles: [] },
|
|
isEdgeRuntime: true,
|
|
navigationContext: null,
|
|
rscStream: createStream(["flight"]),
|
|
ssrHandler: {
|
|
async handleSsr() {
|
|
return createStream(["<html>page</html>"]);
|
|
},
|
|
},
|
|
status: 200,
|
|
});
|
|
|
|
expect(response.headers.get("x-edge-runtime")).toBe("1");
|
|
});
|
|
|
|
it("omits the `x-edge-runtime` marker on HTML stream responses for nodejs-runtime routes", async () => {
|
|
const response = await renderAppPageHtmlResponse({
|
|
clearRequestContext: vi.fn(),
|
|
fontData: { links: [], preloads: [], styles: [] },
|
|
navigationContext: null,
|
|
rscStream: createStream(["flight"]),
|
|
ssrHandler: {
|
|
async handleSsr() {
|
|
return createStream(["<html>page</html>"]);
|
|
},
|
|
},
|
|
status: 200,
|
|
});
|
|
|
|
expect(response.headers.get("x-edge-runtime")).toBeNull();
|
|
});
|
|
});
|