test: add unit tests for Pages Router api-handler

[NathanDrake2406]

Summary

• Adds 34 unit tests for handleApiRoute() in server/api-handler.ts
• Tests body parsing (JSON, form-urlencoded, malformed JSON fallback, empty, unknown content-type)
• Tests MAX_BODY_SIZE enforcement (rejects >1MB with 413)
• Tests cookie parsing edge cases (multiple cookies, = in values, missing header)
• Tests res.status() chaining, res.json(), res.send() type inference, res.redirect() default 307 vs custom
• Tests error handling: missing/non-function default export, thrown errors, ssrFixStacktrace called
• Tests query string + dynamic param merging including duplicate key promotion to arrays

Uses a minimal ViteDevServer mock (only ssrLoadModule + ssrFixStacktrace) and lightweight IncomingMessage/ServerResponse mocks via PassThrough streams.

Test plan

• npx vitest run tests/api-handler.test.ts — 34/34 passing
• CI: Lint, Typecheck, Vitest, Playwright E2E

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@318

commit: fbd7d1a

[james-elicx]

/bigbonk review

[ask-bonk]

Good test suite — clean structure, well-organized sections, sensible mocks. 34 tests covering the main code paths in api-handler.ts is solid. A few observations and suggestions below, mostly minor.

The .gitignore addition for .worktrees/ is fine.

Overall this is ready to merge with one comment fix and optionally a couple of missing coverage items worth considering.

[ask-bonk]

The MAX_BODY_SIZE test duplicates the full mockReq inline construction (lines 297-314) instead of reusing the mockReq() helper. The reason is that mockReq pushes the body eagerly via queueMicrotask, so you need manual stream control for chunked pushing.

Consider factoring out the Object.assign(stream, { ... }) boilerplate into a helper like mockReqStream() that returns both the stream and the typed req, so the MAX_BODY_SIZE test stays DRY:

function mockReqStream(method: string, url: string, headers: Record<string, string> = {}) {
  const stream = new PassThrough();
  const req = Object.assign(stream, {
    method, url, headers: { ...headers },
    httpVersion: "1.1", httpVersionMajor: 1, httpVersionMinor: 1,
    complete: false, connection: null, socket: null, aborted: false,
    rawHeaders: [] as string[], trailers: {} as Record<string, string | undefined>,
    rawTrailers: [] as string[], statusCode: undefined, statusMessage: undefined,
  }) as unknown as http.IncomingMessage;
  return { stream, req };
}

Then mockReq can also use this internally. Not blocking, but it'd make the test easier to maintain.

[ask-bonk]

Nit: http is imported as a default import but only used for the type http.IncomingMessage and http.ServerResponse. Since these are only used as type annotations (in the return types of mockReq/mockRes), you could use:

import type { IncomingMessage, ServerResponse } from "node:http";

and reference them directly, avoiding importing the entire http module at runtime. This is cosmetic — no functional impact.

[ask-bonk]

A few coverage gaps worth considering for follow-up (not blocking this PR):

1. Async handler — all handler mocks are synchronous. The source await handler(apiReq, apiRes) handles async handlers too. Adding one test with a handler that returns a Promise would cover that path.

2. ssrLoadModule failure — if ssrLoadModule rejects (e.g., syntax error in the API route file), the catch block handles it. No test covers this.

3. content-type with charset — e.g., application/json; charset=utf-8. The source uses .includes() matching so it works, but a test would document that behavior.

4. res.json() setting statusCode — currently only tested via res.status(201).json(...). A standalone res.json() without res.status() that verifies the default 200 status code would be explicit.

These are all nice-to-haves, not blockers.

[ask-bonk]

Review posted successfully. Here's a summary of the feedback:

Overall: Good test suite, ready to merge with one fix.

Comments posted:

1. Misleading test title (line 565) — The test says "sends null as JSON" but the assertions verify text/plain behavior. Suggested fixing the title to match the actual behavior being tested.

2. DRY suggestion for MAX_BODY_SIZE test (line 289) — The oversized body test duplicates the full mock request construction inline instead of reusing mockReq(). Suggested extracting a mockReqStream() helper that returns both the stream and req, which both mockReq and the chunked test could use.

3. Import style nit (line 15) — http is imported as a default runtime import but only used for types. Could use import type { IncomingMessage, ServerResponse } instead.

4. Coverage gaps for follow-up (line 789) — Noted four optional additions: async handlers, ssrLoadModule rejection, content-type with charset, and standalone res.json() default status verification. All non-blocking.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Solid test suite. 34 tests covering the key code paths in api-handler.ts — body parsing, cookie parsing, res extensions, query/param merging, MAX_BODY_SIZE enforcement, and error handling. The mocks are well-constructed and the test structure is clean.

All 34 tests pass locally. A few minor observations below, none blocking.

[ask-bonk]

Nit: http is imported as a value import but only used for types (http.IncomingMessage, http.ServerResponse in the return types and casts). Using a type-only import avoids pulling in the module at runtime:

Suggested change

	import http from "node:http";
	import type { IncomingMessage, ServerResponse } from "node:http";

Then replace http.IncomingMessage → IncomingMessage and http.ServerResponse → ServerResponse throughout. Cosmetic only.

[ask-bonk]

The inline Object.assign(stream, { ... }) block duplicates the same property set from mockReq (lines 33-49). Consider extracting a shared mockReqStream() helper that returns { stream, req } so both mockReq and this test can reuse it. The only difference is that mockReq eagerly pushes the body via queueMicrotask while this test needs manual stream control — a clean factoring would separate stream construction from body injection.

Not blocking, but it would make future maintenance easier if the mock properties need updating.

[ask-bonk]

Minor fidelity gap: the mock end() only accepts string, but the real ServerResponse.end() also accepts Buffer and an encoding argument. This works fine for the current tests since api-handler.ts only ever calls end() with strings, but worth noting in case the source adds Buffer support later.

Also, calling end() with no data (as res.redirect() does) leaves _body as "", which works but means tests can't distinguish between "end called with empty string" and "end called with no argument". Not an issue today since no test checks for that distinction.

[ask-bonk]

A couple of additional coverage items worth considering as follow-ups (all non-blocking):

1. Async handler — all handler mocks are synchronous. Adding one test with async (req, res) => { await somePromise; res.json(...) } would exercise the await handler(apiReq, apiRes) path in the source.

2. ssrLoadModule rejection — if ssrLoadModule throws (e.g., syntax error in the API route file), the catch block handles it. No test covers this path.

3. res.send() with undefined — String(undefined) produces "undefined", which is probably not ideal behavior. A test documenting this would be useful, even if the source doesn't handle it specially today.

4. res.send() with arrays — arrays are objects and would take the JSON branch. A test for res.send([1, 2, 3]) would document that behavior explicitly.

[ask-bonk]

Review posted successfully. Here's a summary of what I found:

Verdict: Approved — this is a well-structured test suite ready to merge.

What's good:

• 34 tests all passing, covering the main code paths in api-handler.ts
• Clean test organization with well-separated describe blocks
• Smart use of PassThrough streams to mock IncomingMessage with async body delivery
• Good edge case coverage: malformed JSON fallback, cookies with = in values, null handling in res.send(), duplicate query key promotion

Comments posted (all non-blocking):

1. Import style (line 15) — import http from "node:http" is a runtime import used only for types; could be import type { IncomingMessage, ServerResponse }
2. DRY opportunity (lines 297-314) — the MAX_BODY_SIZE test duplicates the mock construction inline; extracting a shared mockReqStream() helper would reduce duplication
3. Mock fidelity note (line 96) — end() only handles strings, which is fine for current usage but worth noting
4. Coverage follow-ups (line 789) — suggested async handlers, ssrLoadModule rejection, res.send(undefined), and res.send([array]) as future additions

github run