Skip to content

fix(testing): dispatch json nested-Promise guard + scoped routeMap for flight primitives (#582) - #583

Merged
ivogt merged 1 commit into
mainfrom
fix/testing-primitive-parity-582
Jun 14, 2026
Merged

ivogt merged 1 commit into
mainfrom
fix/testing-primitive-parity-582

Conversation

@ivogt

@ivogt ivogt commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Addresses two @rangojs/router/testing parity gaps from #582 (the testing-refinements split from #572). Both are cases where a testing primitive diverged from production, so a test could pass while the real router would behave differently — defeating the dogfood guarantee.

1. dispatch() json route lacked production's nested-Promise guard

Problem (consumer-observable): A json() response-route handler with a forgotten await returns a nested unresolved Promise. Production (response-route-handler.ts) serializes with a replacer that throws RESPONSE_NOT_SERIALIZABLE → a problem+json 500. dispatch() did a bare JSON.stringify, which serializes a Promise to {} — so the test went green with {"data":{}} while production 500s.

// A handler with a forgotten await:
path.json("/api/thing", () => ({ data: fetchSomething() }))  // missing await

// Before: dispatch(...) -> 200 {"data":{}}   (green, but a lie)
// After:  dispatch(...) -> 500 application/problem+json, code RESPONSE_NOT_SERIALIZABLE
//         — exactly what the real router returns.

Fix: extracted production's replacer into a shared lightweight module rsc/json-route-result.ts and routed BOTH production and dispatch through it, so they cannot drift. dispatch's existing catch already maps the thrown RouterError to the same problem+json 500.

Why its own module, not response-route-handler.ts: importing that module into dispatch dragged its heavy runtime graph (cache scope, pattern matching) which transitively reaches Vite virtual modules — breaking a plain (non-Vite) vitest import. Caught in-loop: run-in-request-context.test.ts went red with Received protocol 'virtual:'. The dedicated module depends only on errors.js.

2. renderToFlightString / renderServerTree leaked ctx.reverse to the global route map

Problem: these primitives rebuilt ctx.reverse against getGlobalRouteMap(), so a server component that calls ctx.reverse()/href() was order-dependent on whatever router registered last in the test process. renderHandler already has a scoped routeMap option; the flight primitives did not.

// After: scoped, deterministic reverse — independent of global registration order.
await renderToFlightString(<ProductLink />, {
  routeMap: { product: "/products/:id" },
});

Fix: added a routeMap?: Record<string, string> option (mirroring renderHandler), threaded through setRequestContextParams(params, routeName, routeMap?). Backward compatible — every production caller omits it and keeps getGlobalRouteMap().

Semantics

Primitive Before After
dispatch json + nested Promise 200 {"data":{}} (silent) 500 problem+json RESPONSE_NOT_SERIALIZABLE (= production)
renderToFlightString/renderServerTree ctx.reverse global map (order-dependent) scoped to routeMap option when provided; global otherwise
production response routes unchanged unchanged (same shared guard / getGlobalRouteMap() default)

Test plan

  • Unit, red-before-green: dispatch json route returning a nested Promise → problem+json 500 (not a green {}); proven red against the pre-fix bare JSON.stringify.
  • RSC (dev/prod-agnostic, react-server project): renderToFlightString AND renderServerTree ctx.reverse() scoped to a routeMap resolves /scoped/products/:id; proven red (Unknown route: "product") against the empty global map without the pass-through.
  • Regression caught & fixed in-loop: the virtual: import leak (run-in-request-context.test.ts).
  • typecheck, test:unit:all (incl. RSC suites — test:unit:rsc runs both flight files), lint, format.

Notes / deferred (still in #582)

@pkg-pr-new

pkg-pr-new Bot commented Jun 14, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@rangojs/router@583

commit: 7553b6a

…r flight/handler primitives (#582)

Two `@rangojs/router/testing` parity gaps from #582 (split from #572).

1. dispatch() json route lacked production's nested-Promise guard. Production
   (response-route-handler) serializes a json() result with a replacer that throws
   RESPONSE_NOT_SERIALIZABLE on a nested unresolved Promise (the forgotten-await
   footgun: `() => ({ data: fetchSomething() })`); dispatch did a bare
   JSON.stringify and shipped `{"data":{}}` green while production 500s. Extracted
   the replacer into a shared lightweight module (rsc/json-route-result.ts,
   depends only on errors.js) used by BOTH, so a dispatch json test fails exactly
   where production fails. dispatch's catch already maps the thrown RouterError to
   the same problem+json 500.

   The helper lives in its OWN module, not response-route-handler.ts: importing
   the latter into dispatch dragged its heavy runtime graph, which transitively
   reaches a Vite virtual module and breaks a plain (non-Vite) vitest import
   (caught: run-in-request-context.test.ts went red).

2. The testing primitives leaked ctx.reverse to the GLOBAL route map
   (order-dependent on whatever router registered last). Added a scoped `routeMap`
   option to renderToFlightString/renderServerTree (mirroring renderHandler),
   threaded through setRequestContextParams(params, routeName, routeMap?) --
   backward compatible (production callers omit it and keep getGlobalRouteMap()).
   renderHandler now also passes routeMap to setRequestContextParams, so a nested
   server component reading getRequestContext().reverse scopes to the same map as
   the handler's ctx.reverse -- all three primitives now scope reverse uniformly.

Tests (red-before-green): a dispatch json route returning a nested Promise -> a
problem+json 500 (not a green {}); ctx.reverse() scoped to a provided routeMap
under renderToFlightString, renderServerTree, AND a nested server component in
renderHandler (vs "Unknown route" against the empty global map). Docs: flight
skill routeMap option, feature-file-map new module.

Out of #582 this PR: item 8 (renderRoute synthetic-id stamping) is already
addressed by #581 via a stable WeakMap + a documented input-mutation contract;
item 9 (dispatch executionContext=undefined) is correct for an in-process harness
with no platform execution context. Cross-seam + self-review items remain in #582.

Full gate green: typecheck, test:unit:all (incl. RSC suites), lint, format.
@ivogt
ivogt force-pushed the fix/testing-primitive-parity-582 branch from d108a9a to 7553b6a Compare June 14, 2026 13:37
@ivogt

ivogt commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator Author

/simplify cleanup pass (4 parallel review agents: reuse, simplification, efficiency, altitude). Three angles came back clean — the diff is a de-duplication (extracting the inline json serializer into a shared module) with no efficiency regression and no simpler form. The altitude agent surfaced one real completion, now applied:

  • renderHandler reverse now scopes uniformly. This PR added a routeMap param to setRequestContextParams, but renderHandler was still calling it without routeMap (it only scoped the handler ctx via createHandlerContext). So a nested server component reading getRequestContext().reverse fell back to the global map, while the same component under renderToFlightString scoped. Passing opts.routeMap through makes all three primitives (renderHandler / renderToFlightString / renderServerTree) scope reverse identically. Added a red-before-green RSC test (nested server component's reverse resolves the scoped map; red Unknown route: "product" without the fix).
  • Comment accuracy: softened json-route-result.ts's rationale to not name a specific (unverified) sub-module as the virtual-module culprit — the empirical fact (importing response-route-handler into a plain vitest fails on virtual:) stands; the exact edge does not need over-attribution.

Skipped (noted, not applied): the repeated thenable duck-check idiom (no exported helper exists; codebase-wide, out of scope) and a top-of-file overview comment that is still accurate.

Full gate re-run green: typecheck, test:unit:all (incl. RSC), lint, format.

@ivogt
ivogt merged commit c8e2d5d into main Jun 14, 2026
25 checks passed
@ivogt
ivogt deleted the fix/testing-primitive-parity-582 branch June 14, 2026 13:55
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