test: add unit tests for next/document shim by NathanDrake2406 · Pull Request #319 · cloudflare/vinext

NathanDrake2406 · 2026-03-07T14:48:34Z

Summary

Adds 5 focused tests for the next/document shim components (Main, NextScript, Head, default Document)
Tests the contracts the Pages Router dev-server depends on, not React rendering:
- Main renders id="__next" with __NEXT_MAIN__ placeholder (dev-server replaces with page content)
- NextScript renders  comment (dev-server replaces with hydration scripts)
- Head injects default charset + viewport meta, preserves custom children
- Default Document assembles all sub-components in the correct nesting order (head < body < main < scripts)

Test plan

npx vitest run tests/document.test.ts — 5/5 passing
CI: Lint, Typecheck, Vitest, Playwright E2E

pkg-pr-new · 2026-03-07T14:49:08Z

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

commit: c43b529

james-elicx · 2026-03-07T15:11:21Z

/bigbonk review

ask-bonk

Good PR — the tests are well-structured, correctly target the placeholder contracts the dev-server relies on, and all 5 pass. A couple of observations below, nothing blocking.

ask-bonk · 2026-03-07T15:12:58Z

+describe("Head", () => {
+  it("injects default charset and viewport meta tags", () => {
+    const html = render(React.createElement(Head));
+    expect(html).toContain('charSet="utf-8"');


This works today because React's renderToString preserves the JSX prop name charSet (camelCase) in the output HTML attribute. If React ever normalizes this to the standard HTML charset (lowercase), this assertion would break. Not a real concern for now since React has been consistent about this, but worth noting.

If you wanted to be resilient, you could match the actual <meta> element more broadly, e.g.:

Suggested change

expect(html).toContain('charSet="utf-8"');

expect(html).toMatch(/charset/i);

expect(html).toContain('content="width=device-width, initial-scale=1"');

That said, matching exactly what the source renders is also a reasonable choice — you'd want the test to break if React changes the output, so you can update the shim accordingly.

ask-bonk · 2026-03-07T15:13:07Z

Review posted. Here's a summary of my feedback on PR #319:

Overall: Good PR. The tests are well-structured, correctly verify the placeholder contracts that dev-server.ts, index.ts, and static-export.ts all depend on (the __NEXT_MAIN__ and  string replacements), and all 5 tests pass. Nothing blocking.

Comments posted:

Missing Html coverage — The Html component is exported from the document shim but isn't tested. A simple test for lang prop forwarding would complete the API surface coverage.
Unrelated .gitignore change — The .worktrees/ entry is unrelated to the document shim tests. Harmless, but ideally in a separate commit.
charSet assertion fragility — The test matches charSet="utf-8" (React's camelCase output). Noted this is reasonable since you'd want the test to break if React changes behavior, but flagged for awareness.
Structural completeness — The index-based ordering check is solid. Suggested optionally verifying the output starts with <html> to confirm Html is the root wrapper.

github run

Cover Html, Head, Main, NextScript, and default Document with renderToString assertions for attributes, children, placeholder content, and correct nesting order.

james-elicx · 2026-03-07T16:09:30Z

/bigbonk review

ask-bonk

Clean, well-scoped test file. The tests correctly verify the placeholder contracts that dev-server.ts, index.ts, and static-export.ts all depend on for string replacement. All 7 tests pass. The structural ordering test in "Default Document" is particularly valuable — it would catch regressions in component nesting that would silently break SSR output.

A couple of minor observations below, none blocking.

ask-bonk · 2026-03-07T16:11:50Z

+describe("Head", () => {
+  it("injects default charset and viewport meta tags", () => {
+    const html = render(React.createElement(Head));
+    expect(html).toContain('charSet="utf-8"');


The charSet="utf-8" assertion is correct — that's what React 18/19's renderToString produces for <meta charSet="utf-8" />. This is the right thing to assert against since the source shim (document.tsx:29) uses the JSX charSet prop, and renderToString preserves it as-is in the attribute output.

The prior review suggested matching case-insensitively to be resilient against React changes, but I agree with the PR author's approach: if React ever changes how it serializes this attribute, we'd want the test to fail so we can investigate whether the dev-server's string replacements are affected.

ask-bonk · 2026-03-07T16:11:50Z

+  it("renders the __NEXT_SCRIPTS__ comment that dev-server replaces with hydration scripts", () => {
+    const html = render(React.createElement(NextScript));
+    // Dev-server replaces this HTML comment with __NEXT_DATA__ + module script tags
+    expect(html).toContain("<!-- __NEXT_SCRIPTS__ -->");


Worth noting: the dev-server replaces the exact string "" (with the space padding inside the comment). If someone ever changed the shim to use  (no spaces), the dev-server would silently stop replacing it. This test correctly pins the exact format, which is good — it would catch that kind of drift.

ask-bonk · 2026-03-07T16:11:50Z

+    expect(bodyOpen).toBeLessThan(mainDiv);
+    expect(mainDiv).toBeLessThan(placeholder);
+    expect(placeholder).toBeLessThan(scripts);
+    expect(scripts).toBeLessThan(bodyClose);


Minor: this checks that __NEXT_SCRIPTS__ comes before </body>, but doesn't assert that </body> itself is present (only that indexOf returns something). If </body> were somehow missing, indexOf would return -1, and scripts < -1 would fail — so it does catch it indirectly. Just noting for clarity.

Also, the bodyClose variable is declared on line 72 but only used here. Since it's only needed for one assertion, you could inline it, but the current style is fine for readability.

ask-bonk · 2026-03-07T16:12:00Z

Review submitted. Here's a summary of my review on PR #319:

Verdict: Approved

The PR adds a well-structured test file (tests/document.test.ts) with 7 tests covering all exported components from the next/document shim. The tests correctly verify the placeholder contracts that 4 different consumer files depend on (dev-server.ts, index.ts, static-export.ts, and the production virtual module).

Key observations in my review:

charSet assertion — Confirmed the charSet="utf-8" assertion is correct and should remain exact-match (not case-insensitive) since we'd want tests to break if React changes serialization behavior.
Exact placeholder format — Noted that the  test correctly pins the exact comment format including spaces, which is important because the dev-server does exact string replacement.
bodyClose assertion — Minor note that the </body> presence check works indirectly through the ordering assertion (would fail if indexOf returned -1).

The .gitignore change mentioned in the prior review is not actually in this PR's diff — the diff only contains the new test file.

github run

ask-bonk Bot reviewed Mar 7, 2026

View reviewed changes

test: add unit tests for next/document shim components

5d79dc1

Cover Html, Head, Main, NextScript, and default Document with renderToString assertions for attributes, children, placeholder content, and correct nesting order.

NathanDrake2406 force-pushed the test/document-shim-coverage branch from 1928625 to 5d79dc1 Compare March 7, 2026 15:42

ask-bonk Bot approved these changes Mar 7, 2026

View reviewed changes

james-elicx merged commit 048c49f into cloudflare:main Mar 7, 2026
25 of 26 checks passed

NathanDrake2406 deleted the test/document-shim-coverage branch March 8, 2026 07:02

	expect(html).toContain('charSet="utf-8"');
	expect(html).toMatch(/charset/i);
	expect(html).toContain('content="width=device-width, initial-scale=1"');

Conversation

NathanDrake2406 commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

pkg-pr-new Bot commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

james-elicx commented Mar 7, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ask-bonk Bot Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ask-bonk Bot commented Mar 7, 2026

Uh oh!

james-elicx commented Mar 7, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Mar 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NathanDrake2406 commented Mar 7, 2026 •

edited

Loading

pkg-pr-new Bot commented Mar 7, 2026 •

edited

Loading