Skip to content

fix(router): dispatch() classifies through the shared request stages (S14b) - #1252

Merged
ivogt merged 1 commit into
reorg/stackfrom
reorg/s14b-dispatch-classify
Oct 23, 2026
Merged

ivogt merged 1 commit into
reorg/stackfrom
reorg/s14b-dispatch-classify

Conversation

@ivogt

@ivogt ivogt commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

devcorner: rango-t159 · session rango-s300

dispatch() now classifies each request with the production handler's classify stage and answers its terminal plans inside the global middleware chain. A stale or cross-app navigation reloads, a rewritten action fails with production's error, and global middleware sees the route only after next(), all as router.fetch does. Follow-up to S14 (#1242). It must land before the S22 pilot (rango-t150).

Problem

S14 moved the handler's request stages into rsc/request-stages.ts, but testing/dispatch.ts still matched through router.previewMatch, because its suites used a stub router. That left four cases where a test passed under dispatch() but production answered differently:

  1. A navigation carrying another deploy's _rsc_v (version mismatch), or another router's _rsc_rid (app switch), never reloaded. On a component route dispatch() threw "does not render RSC routes", and on an unmatched path it answered 404. Production sends X-RSC-Reload.
  2. An action (?_rsc_action) that global middleware rewrote to a response route was served by that route. Production throws A rewrite of a server action or form post must target a page route; … is a response route.
  3. Route params and name were set before the global chain, so global middleware saw ctx.routeName before next(). Production sets them inside the chain, in executeRequest, so they are undefined there.
  4. After a rewrite, a response route's request.timeout carried the pass request's telemetry id. Without a request-id header that id differed from dispatch()'s own request.start/request.end. This regressed in S14.

A test moved from e2e into dispatch() (the S22 plan) could therefore pass where production reloads, throws, or reads the route later.

Before / after

The same test, written against dispatch():

setBuildVersions({ data: "d2", document: "h2" });
const seen: unknown[] = [];
const router = createRouter({})
  .use(async (ctx, next) => {
    seen.push(ctx.routeName); // read before next()
    await next();
  })
  .routes(
    urls(({ path }) => [
      path("/shop", ShopPage, { name: "shop" }),
      path.json("/api/orders/:id", getOrder, { name: "api.order" }),
    ]),
  );

const nav = await dispatch(router, {
  request: "/shop?_rsc_partial=true&_rsc_v=h1",
}).catch((error: unknown) => error);
await dispatch(router, { request: "/api/orders/7" });

Before:

nav; // Error: dispatch() does not render RSC routes — the route matched at "/shop" …
seen; // ["api.order"]

After (as router.fetch answers):

(nav as Response).headers.get("x-rsc-reload"); // "http://localhost/shop"
seen; // [undefined, undefined]: the reload ran inside the global chain, and the route is set after next()

Consumer example

import { dispatch, setBuildVersions } from "@rangojs/router/testing";
import { createRouter, rewrite, urls } from "@rangojs/router";

const router = createRouter({ id: "shop-app" })
  .use(async (ctx, next) => {
    await next();
    // the route is known only after next(), as in production
    audit(ctx.routeName);
  })
  .use("/api/v1/orders", () => {
    throw rewrite("/api/v2/orders");
  })
  .routes(
    urls(({ path }) => [
      path("/shop", ShopPage, { name: "shop" }),
      path.json("/api/v1/orders", listOrdersV1),
      path.json("/api/v2/orders", listOrders, { name: "api.orders" }),
    ]),
  );

it("reloads a tab left open across a deploy", async () => {
  setBuildVersions({ data: "d2", document: "h2" });
  const res = await dispatch(router, {
    request: "/shop?_rsc_partial=true&_rsc_v=h1",
  });
  expect(res.headers.get("x-rsc-reload")).toBe("http://localhost/shop");
});

it("reloads a navigation that crossed in from another app", async () => {
  const res = await dispatch(router, {
    request: "/shop?_rsc_partial=true&_rsc_rid=blog-app",
  });
  expect(res.headers.get("x-rsc-reload")).toBe("http://localhost/shop");
});

it("refuses an action the API move would send to a response route", async () => {
  await expect(
    dispatch(router, {
      request: new Request(
        "http://localhost/api/v1/orders?_rsc_action=orders%23save",
        { method: "POST" },
      ),
    }),
  ).rejects.toThrow("must target a page route");
});

Semantics

Request through dispatch() Before After (= production)
_rsc_v is not the router's document version, component route throws "does not render RSC routes" X-RSC-Reload at the visitor's URL
same, unmatched path 404 X-RSC-Reload
same, response route, request that is not a partial the route's response X-RSC-Reload (an action with a same-origin Referer reloads to the referer)
_rsc_partial with another router's _rsc_rid, component route or unmatched path throws / 404 X-RSC-Reload
_rsc_partial to a response route (stale _rsc_v, foreign _rsc_rid, or neither) X-RSC-Reload unchanged
_rsc_action a rewrite moved to a response route, a redirect or an unmatched path serves the target / 308 / 404 rejects with A rewrite of a server action or form post must target a page route; …
global middleware reads ctx.routeName / request-context params before next() the route undefined / {}
the same after next() the route unchanged
the same after next() on a trailing-slash redirect (308) the matched route undefined / {}
response-route timeout on a rewritten pass, no request-id header request.timeout id ≠ request.start (since S14) same id as request.start / request.end (as in 0.27.0)
component route without _rsc_v / _rsc_rid throws before any middleware unchanged: same error, same point
dispatch() options, errors, DispatchOptions — unchanged

createRSCHandler behaviour is unchanged.

Code change

Internal shape before:

dispatch: findMatch + previewMatch (pre-chain) -> beforePass sets params -> core: 308 | serveResponseRoute | 404
handler:  core: classify -> terminalPlanResponse -> origin guard -> executeRequest (sets params) -> ...

After:

dispatch: classify (pre-chain; component plan throws) -> core: terminalPlanResponse -> recordPlanRoute -> 308 | serveResponseRoute | 404
handler:  core: classify -> terminalPlanResponse -> origin guard -> executeRequest (recordPlanRoute) -> ...
  • src/testing/dispatch.ts: matchDispatchPass (findMatch + previewMatch) is replaced by classifyPass: stages.classify, then the RSC-route throw for any plan an exhaustive ANSWERS_MODE table (Record<RequestPlan["mode"], boolean>) marks as rendering a component, unless isUnmatchedPlan. The core runs terminalPlanResponse and recordPlanRoute, then the 308, serveResponseRoute or the 404. onHop gives a rewritten pass's request the visitor's telemetry id. The stub-shaped previewMatch member is gone from DispatchableRouter; findMatch takes the classify type.
  • src/rsc/request-stages.ts: recordPlanRoute (params, route name and _classifiedRoute, skipped on a redirect), moved out of executeRequest. isUnmatchedPlan. classify's not-found fallback sets unmatched: true. partialRedirectResponse is no longer exported; its only caller is terminalPlanResponse.
  • src/rsc/handler.ts: executeRequest calls recordPlanRoute. This is a synchronous move, so it adds no microtask.
  • src/router/request-classification.ts: FullRenderPlan.unmatched?: true.
  • src/router/telemetry.ts: setRequestId(request, id).
  • src/router/preview-match.ts, match-handlers.ts, router-interfaces.ts: previewMatch no longer returns responseType, handler, negotiated or manifestEntry. dispatch was their last reader. prerender/build-shell-capture.ts reads only routeKey, params and routeMiddleware, and a winning variant's route middleware is kept.
  • Why classify runs before the chain in dispatch and inside it in the handler: the component-route throw is this primitive's hard boundary. It has to come before any middleware side effect and before request.start, as it did. The plan is classified once per pass and reused by the core. previewMatch ran at the same point before this change.

Source, without tests: 8 files, +158 / −203.

Test plan

  • New userland suite src/testing/__tests__/dispatch-request-plans.test.ts, through @rangojs/router/testing (dispatch, setBuildVersions). Each test is red on the base (4a77cd3) and green here. On the base all 6 failed: the two reload tests threw "does not render RSC routes", the two timing tests saw a route name where production has undefined, the action test resolved 200 instead of rejecting, and the timeout test got requestId t-2 against request.start's t-1.
    • "reloads a navigation from a tab on another deploy at its URL, inside the global chain": _rsc_v reload, and global middleware runs around it.
    • "reloads a navigation that crossed in from another app": _rsc_rid reload.
    • "global middleware sees the route after next(), not before it": params and name timing.
    • "a trailing-slash redirect sets no route".
    • "fails an action a global middleware moved to a response route, and never runs its handler": the rewritten-action rule.
    • "a response-route timeout after a rewrite carries the request's own requestId": one id across request.start, request.timeout and request.end.
  • Dispatch suites keep their assertions. The only setup change is in dispatch.test.ts "Vary: Accept on negotiated response routes": createRouter with json and text routes on one path replaces the previewMatch stub, and a stale previewMatch test title is renamed. The adapted describe passes on the base too.
  • pnpm run test:unit:all: router 566 files / 9,354 passed (5 skipped, 6 todo); router rsc 84 / 1,067 passed (3 expected fail); cloudflare-basic 18 / 90 + 13 / 39; vite-rsc-demo 6 / 27 + 1 / 1; mini 8 / 25 + 2 / 3; shared-e2e 4 / 20. This includes testing-module-graph.test.ts (the testing barrel and a dispatch load no @vitejs/plugin-rsc) and request-microtasks.rsc-test.tsx, whose counts are unchanged.
  • E2e from packages/rangojs-router, dev and production, --grep "response-handler|content-negotiation|redirect-guard|rewrite", checked with --list first: 86 dev + 79 prod tests across response-handler, content-negotiation, redirect-guard, rewrite, plus the semantic-matrix rewrite rows and one url-hooks title.
    • After the final change: 165 passed, no retries.
    • On the first commit: 162 passed and 3 flaky. Those were dev redirect-guard tests whose fixture beforeAll timed out at 30 s (e2e/fixture.ts:216). They passed on retry, and a --retries=0 rerun of dev redirect-guard passed 9 of 9.
  • pnpm run typecheck (16 of 16), oxlint --deny-warnings, oxfmt --check.
  • CI lint scripts: check-e2e-bucketing, check-e2e-parity --strict, check-e2e-console-baseline, check-suspense-contract, check-no-external-store, node --test tools/*.test.mjs (53), check-doc-links, check-docs-api, check-skills, generate-skills-catalog --check, generate-route-types --check, and check-internals. All OK.

Practices

  • Lint: oxlint --deny-warnings and oxfmt --check are clean. Formatting fixes from oxfmt were applied, and no rule was silenced.
  • Dead code:
    • Removed matchDispatchPass and the DispatchPass type.
    • Removed the previewMatch member from DispatchableRouter.
    • Removed previewMatch's responseType, handler, negotiated and manifestEntry fields, with their two type copies. Every reader was searched across src, tests, e2e and docs; the only remaining caller, build-shell-capture, reads none of them.
    • partialRedirectResponse is no longer exported.
    • Removed the now-unused EntryData imports.
    • Kept on purpose: previewMatch itself, because build-shell-capture uses it.
  • Tests: the 6 new userland tests are red-before-green. The touched suites ran, then test:unit:all once, then the targeted e2e in dev and production.
  • Code review: the diff was reviewed for reuse, simplification, efficiency and altitude, and the fixes are applied: the shared recordPlanRoute stage, the explicit unmatched marker, the exhaustive mode table, setRequestId from onHop, the RequestStages alias, and the previewMatch trim. Left, with reasons:
    • DispatchableRouter still declares its stage fields by hand rather than deriving them from StageRouter. That duplication predates this change.
    • The inline pluginRscMock follows this directory's convention.
    • The rewritten-pass telemetry test stays in the new suite, so S14b's contract lives in one file.
    • classifyRequest signals a 404 with a throw and catch. Changing that changes the production path.
    • A plain GET to a response route now loads its manifest under the document (isSSR) partition, as the handler does.
    • A correctness review of the final commit found 7 items, none needing a change; each is listed under Notes.
  • Docs: CHANGELOG ## Unreleased has a new entry, "Changed: dispatch() classifies a request as production does". S14's timeout bullet is corrected, since the requestId again matches request.start and request.end. Also updated: skills/testing/response-routes.md (stages, reloads, the rewritten-action rule, the routeName timing, the requestId on timeouts), docs/testing.md (dispatch section) and docs/internal/feature-map.md (testing entry, timeout entry). node tools/check-skills.mjs passes. No source file was added or removed, so feature-file-map.md is unchanged.

Notes

  • Telemetry ids in production: getRequestId keys by Request object (router/telemetry.ts), and applyRewrite builds a new Request per hop (rsc/rewrite-passes.ts). router/logging.ts keeps a second per-Request registry. A header-less rewritten request therefore gets a new id per pass in production too. This PR does not change that, because it would change production telemetry. I have not shown an event pair within one production request that splits across passes, so it is recorded here and not filed as a defect.
  • handleResponseRoute's parameter is still named preview (rsc/response-route-handler.ts). That naming predates this change.
  • Timing: the production handler change is a synchronous function extraction, and request-microtasks.rsc-test.tsx is unchanged. Dispatch does one findMatch per pass, where it used to do two.
  • Correctness review of the final commit. There were 7 findings, and none required a code change.
    • A rewrite onto a redirect target keeping the previous pass's params: not possible. rebaseRequestContext resets params, routeName, reverse and _classifiedRoute on every hop (server/request-context.ts).
    • id: routerId ?? "" for a router without an id: every router createRouter builds has one (RangoInternal.id: string), and ensureRouterTrie already returns early without one.
    • The request.timeout id differs from production: by design, it is ticket item 4. Production emits no request.start/request.end for a response route, and dispatch()'s own lifecycle events need one id (see the first note).
    • A classify error on the visitor's URL escapes before request.start, while on a rewrite target it emits request.error: unchanged from before, since previewMatch ran at the same two points.
    • unmatched?: true widens the production plan type for one reader: kept. It makes classify's 404 fallback explicit where it was a null as any the types hid. The handler ignores it.
    • previewMatch/resolveRoute comments naming manifestEntry and responseType: still accurate, because negotiateRoute reads both.
    • A component visitor URL that global middleware would rewrite still throws before the middleware runs: the documented boundary, kept as the ticket requires.

…ages (S14b)

dispatch matched through router.previewMatch, so it skipped production's
terminal plans and set route params before the global chain. It now
classifies each pass with the shared classify stage and answers
terminalPlanResponse inside the global chain, as createRSCHandler does:

- a `_rsc_v` version mismatch and a `_rsc_rid` app switch reload;
- an action a rewrite moved to a response route, a redirect or an
  unmatched path throws production's error;
- params, route name and the classified snapshot are recorded inside the
  chain by recordPlanRoute, a stage executeRequest now runs too, and not
  on a trailing-slash redirect;
- a rewritten pass's request takes the visitor's telemetry id
  (setRequestId, from onHop), so its request.timeout matches dispatch's
  own request.start and request.end.

classify's not-found fallback carries `unmatched: true`. The
component-route throw stays before any middleware. previewMatch drops the
response-route fields no caller reads any more. The negotiation tests
build a real router instead of a previewMatch stub. New userland suite:
testing/__tests__/dispatch-request-plans.test.ts.
@ivogt
ivogt merged commit 421bb71 into reorg/stack Oct 23, 2026
19 checks passed
@ivogt
ivogt deleted the reorg/s14b-dispatch-classify branch October 23, 2026 20:10
ivogt added a commit that referenced this pull request Oct 23, 2026
Brings in S10b (#1254), S6 (#1250), S9 (#1244) and S14b (#1252).

Conflicts: docs/internal/feature-file-map.md and feature-map.md. Both
keep reorg/stack's text plus this branch's two facts: the cache row's
CFCacheStore sentence (readL1, readKv, the family descriptors, the read
bench) and the "Cloudflare store family reads" bullet, placed after the
Vercel store bullets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant