Skip to content

test: use metadata API in app-basic root layout#316

Merged
james-elicx merged 1 commit intocloudflare:mainfrom
NathanDrake2406:fix/metadata-fixture-title-duplication
Mar 7, 2026
Merged

test: use metadata API in app-basic root layout#316
james-elicx merged 1 commit intocloudflare:mainfrom
NathanDrake2406:fix/metadata-fixture-title-duplication

Conversation

@NathanDrake2406
Copy link
Copy Markdown
Contributor

Summary

  • move the shared app-basic root title from manual <head> markup to the Metadata API
  • enable the metadata-suspense duplicate-title regression test
  • update the test comment to describe the real failure mode

Why

The duplicate <title> was coming from the shared test fixture root layout hardcoding <title>App Basic</title>, not from Suspense-specific metadata streaming.

Testing

  • pnpm test tests/nextjs-compat/metadata-suspense.test.ts tests/nextjs-compat/metadata.test.ts

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Mar 7, 2026

Open in StackBlitz

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

commit: 5119d55

@james-elicx
Copy link
Copy Markdown
Collaborator

/bigbonk review

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean fix. The root cause analysis is correct: the duplicate <title> was from the hardcoded JSX in the root layout conflicting with the Metadata API output, not a Suspense streaming bug.

I verified:

  • tests/app-router.test.ts:120 asserts <title>App Basic</title> — still passes because the Metadata API renders the same title.
  • The removed <meta charSet> and <meta name="viewport"> were redundant — vinext (matching Next.js behavior) auto-injects charset and viewport defaults when using the Metadata API.
  • The suspense test fixture (metadata-suspense-test/page.tsx) exports title: "Suspense Metadata Title", which properly overrides the root layout's "App Basic" through metadata merging — so the single <title> assertion is correct.

Good to have this test running instead of skipped.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Mar 7, 2026

Approved the PR. The change is straightforward and correct:

  • The root cause of the duplicate <title> was properly identified — the hardcoded <title>App Basic</title> in the root layout JSX conflicted with titles rendered via the Metadata API.
  • Switching to export const metadata is the right fix and matches how Next.js apps should declare metadata.
  • The removed <meta charSet> and <meta name="viewport"> are auto-injected by the Metadata API, so no functionality is lost.
  • The existing test at tests/app-router.test.ts:120 that asserts <title>App Basic</title> will continue to pass since the Metadata API renders the same output.
  • Enabling the previously-skipped regression test adds real coverage.

github run

@james-elicx james-elicx merged commit 8b356e4 into cloudflare:main Mar 7, 2026
14 checks passed
@NathanDrake2406 NathanDrake2406 deleted the fix/metadata-fixture-title-duplication branch March 18, 2026 09:51
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.

2 participants