-
Notifications
You must be signed in to change notification settings - Fork 356
fix(app-router): preserve streamed metadata placement parity #2572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
24704d1
dfca031
9f81102
f1bfec3
19cb0b2
8b2ef07
139f303
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,6 +187,35 @@ async function renderRouteEntry(elements: AppElements, routeId: string): Promise | |
| ); | ||
| } | ||
|
|
||
| async function renderRouteDocument(elements: AppElements, routeId: string): Promise<string> { | ||
| const { ElementsContext, Slot } = await import("../packages/vinext/src/shims/slot.js"); | ||
| return renderHtml( | ||
| createElement( | ||
| "html", | ||
| null, | ||
| createElement("head"), | ||
| createElement( | ||
| "body", | ||
| null, | ||
| createElement( | ||
| ElementsContext.Provider, | ||
| { value: elements }, | ||
| createElement(Slot, { id: routeId }), | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| function readDocumentSection(html: string, tagName: "head" | "body"): string { | ||
| const start = html.indexOf(`<${tagName}>`); | ||
| const end = html.indexOf(`</${tagName}>`); | ||
| if (start === -1 || end === -1) { | ||
| throw new Error(`Rendered document is missing <${tagName}>`); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| return html.slice(start, end + tagName.length + 3); | ||
| } | ||
|
|
||
| async function withTimeout<T>(promise: Promise<T>, timeoutMs: number): Promise<T> { | ||
| return new Promise<T>((resolve, reject) => { | ||
| const timeoutId = setTimeout(() => { | ||
|
|
@@ -303,7 +332,14 @@ async function buildGeneratedMetadataRouteHtml( | |
| page: { | ||
| default: PageProbe, | ||
| async generateMetadata() { | ||
| return { title: "generated page" }; | ||
| return { | ||
| title: "generated page", | ||
| alternates: { | ||
| canonical: "https://example.com/generated", | ||
| languages: { "en-US": "https://example.com/en/generated" }, | ||
| }, | ||
| robots: { index: false, follow: false }, | ||
| }; | ||
| }, | ||
| }, | ||
| params: [], | ||
|
|
@@ -327,7 +363,7 @@ async function buildGeneratedMetadataRouteHtml( | |
| htmlLimitedBots, | ||
| }); | ||
|
|
||
| return renderRouteEntry(elements, "route:/generated"); | ||
| return renderRouteDocument(elements, "route:/generated"); | ||
| } | ||
|
|
||
| function RouteLoadingProbe() { | ||
|
|
@@ -418,13 +454,25 @@ describe("app page route wiring helpers", () => { | |
| }); | ||
| }); | ||
|
|
||
| it("renders generated metadata in a hidden body outlet for streaming-capable requests", async () => { | ||
| // Ported from Next.js: test/e2e/app-dir/metadata-streaming/metadata-streaming.test.ts | ||
| // https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/metadata-streaming/metadata-streaming.test.ts | ||
| it("hoists generated metadata into the head for streaming-capable requests", async () => { | ||
| // Next.js renders the same real elements in its streaming metadata outlet: | ||
| // https://github.com/vercel/next.js/blob/canary/packages/next/src/lib/metadata/metadata.tsx | ||
| // Vinext has already resolved metadata before this render, so React can hoist | ||
| // the elements into <head> instead of appending them after a streamed shell. | ||
| const html = await buildGeneratedMetadataRouteHtml("HeadlessChrome"); | ||
|
|
||
| expect(html).not.toContain("<title>generated page</title><div"); | ||
| expect(html).toContain('<div hidden=""><title>generated page</title></div>'); | ||
| const head = readDocumentSection(html, "head"); | ||
| const body = readDocumentSection(html, "body"); | ||
|
|
||
| expect(head).toContain("<title>generated page</title>"); | ||
| expect(head).toContain('rel="canonical" href="https://example.com/generated"'); | ||
| expect(head).toContain('hrefLang="en-US" href="https://example.com/en/generated"'); | ||
| expect(head).toContain('name="robots" content="noindex, nofollow"'); | ||
| expect(body).not.toContain("<title>generated page</title>"); | ||
| expect(body).not.toContain('rel="canonical"'); | ||
| expect(body).not.toContain('hrefLang="en-US"'); | ||
| expect(body).not.toContain('name="robots"'); | ||
| // Metadata is hoisted out, leaving the body outlet empty. | ||
| expect(body).toContain('<div hidden=""></div>'); | ||
| }); | ||
|
|
||
| it("renders generated metadata in the head for configured html-limited bots", async () => { | ||
|
|
@@ -446,12 +494,14 @@ describe("app page route wiring helpers", () => { | |
| }); | ||
|
|
||
| it("falls back to the default html-limited bot list for an empty config string", async () => { | ||
| // Next.js normalizes a falsy htmlLimitedBots config to the default bot regex. | ||
| // Next.js normalizes a falsy htmlLimitedBots config to the default bot regex, | ||
| // so HeadlessChrome is treated as streaming-capable and metadata is hoisted. | ||
| // https://github.com/vercel/next.js/blob/canary/packages/next/src/server/lib/streaming-metadata.ts | ||
| const html = await buildGeneratedMetadataRouteHtml("HeadlessChrome", ""); | ||
|
|
||
| expect(html).not.toContain("<title>generated page</title><div"); | ||
| expect(html).toContain('<div hidden=""><title>generated page</title></div>'); | ||
| expect(html).toContain("<title>generated page</title>"); | ||
| expect(html).toContain('<div hidden=""></div>'); | ||
| expect(html).not.toContain('<div hidden=""><title>generated page</title>'); | ||
| }); | ||
|
|
||
| it("resolves child segments from tree positions and preserves route groups", () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2755,7 +2755,7 @@ describe("MetadataHead rendering", () => { | |
| expect(html).toContain('href="/apple.png"'); | ||
| }); | ||
|
|
||
| it("serializes rich metadata for the streaming body outlet", () => { | ||
| it("serializes rich metadata for compatibility", () => { | ||
| const html = renderMetadataToHtml( | ||
| { | ||
| metadataBase: new URL("https://example.com"), | ||
|
|
@@ -2796,8 +2796,7 @@ describe("MetadataHead rendering", () => { | |
| expect(html).toContain('property="og:image:type"'); | ||
| expect(html).toContain('content="image/png"'); | ||
| expect(html).toContain('rel="alternate"'); | ||
| expect(html).toContain('hreflang="en-US"'); | ||
| expect(html).toContain('href="https://example.com/products/en"'); | ||
| expect(html).toContain('href="https://example.com/products/en" hreflang="en-US"'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assertion change isn't required by this PR. |
||
| expect(html).toContain('rel="icon"'); | ||
| expect(html).toContain('href="/icon.png"'); | ||
| expect(html).toContain('sizes="32x32"'); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
<div hidden>wrapper is now purely vestigial: its metadata children are all hoisted to<head>, leaving an empty<div hidden></div>in the body (as your own test attests/app-page-route-wiring.test.tsasserts). That's harmless and keeps a stable outlet, so no change needed — but worth a one-line comment noting the wrapper is intentionally retained as the streaming outlet placeholder, since a future reader may be tempted to drop it. Related: as the PR body notes, once themetadataPlacementUA gate is removed thisbodybranch collapses into theheadpath entirely.