fix: support static metadata URL resolver (fillStaticMetadataSegment)

[Divkix]

Fixes #861

Summary

• Port fillStaticMetadataSegment from Next.js (commit 7873aea) to correctly resolve URLs for static metadata files under dynamic parent segments
• Update scanMetadataFiles to normalize dynamic segments to "-" placeholders in servedUrl for static metadata files
• Strip route groups and parallel route slots from static metadata URL paths, applying djb2 hash suffixes for collision avoidance
• Keep dynamic metadata files on the existing urlPrefix path for patternParts-based runtime matching
• Add unit tests for fillStaticMetadataSegment and integration tests for static metadata scanning under dynamic parents

Test plan

• Unit tests: tests/metadata-routes.test.ts — 69 tests (8 new for fillStaticMetadataSegment, 3 new for scan with dynamic parents)
• Integration tests: tests/app-router.test.ts — metadata routes section (20 tests, all passing)
• Full suite: 4398 tests passing (1 pre-existing flaky timeout in pages-router)

[pkg-pr-new]

Open in StackBlitz

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

commit: b8d8bb1

[james-elicx]

/bigbonk review

[ask-bonk]

Good work porting fillStaticMetadataSegment from Next.js. The core logic is correct: dynamic segments get - placeholders, route groups and parallel slots are stripped from URLs, and djb2 hash suffixes disambiguate collisions from invisible parents. Tests are thorough and well-structured.

A few issues worth addressing — one correctness nit, one code-health concern, and a couple of minor observations.

[ask-bonk]

Both branches of the inner ternary return "", making it a no-op:

lastSlash === -1 ? "" : ""

This reads like one branch was meant to be something else (maybe the full page string?), or it's just a simplification artifact. Either way it should be collapsed:

Suggested change

	const parentPathname = lastSlash > 0 ? page.slice(0, lastSlash) : lastSlash === -1 ? "" : "";
	const parentPathname = lastSlash > 0 ? page.slice(0, lastSlash) : "";

[Divkix]

Fixed in b8d8bb1 — collapsed the inner ternary since both branches returned empty string.

[ask-bonk]

Nit: the inline IIFE duplicates logic that fillStaticMetadataSegment and its helpers already encapsulate — getStaticMetadataRoute, getMetadataRouteSuffix, suffix application. The only difference is that scanMetadataFiles uses config.urlPath (extensionless, e.g. /icon) while fillStaticMetadataSegment uses the actual filename with extension.

This means:

• fillStaticMetadataSegment is exported but not used by scanMetadataFiles — it's only exercised in tests.
• getMetadataRouteFilename is only called by fillStaticMetadataSegment, so it's also dead code in production.

Consider one of:

1. Refactor so scanMetadataFiles actually calls fillStaticMetadataSegment (or a shared helper), passing config.urlPath instead of the real filename. This eliminates the duplication and ensures the exported function is the one tested and used.
2. If fillStaticMetadataSegment is intended for future use outside of scanning (e.g., runtime resolution), document that intent and add a comment in scanMetadataFiles explaining why it doesn't delegate.

As-is, the inline code and the exported function could drift apart silently.

[Divkix]

Fixed in b8d8bb1 — extracted a shared computeMetadataRouteSuffix helper that both scanMetadataFiles and fillStaticMetadataSegment/`getMetadataRouteFilename` now use. Eliminates the duplicated route+suffix computation.

[ask-bonk]

Minor inconsistency: getMetadataRouteSuffix only exempts sitemap from hash suffixes (line 399), while the existing metadataRouteSuffix (line 454) exempts sitemap, robots, and manifest.

This is inconsequential today because robots and manifest are nestable: false (so they only appear at root, where there are no invisible parents). But if nestable ever changed, the two functions would disagree. Worth either:

• Adding the same exemptions here for defensive consistency, or
• Adding a brief comment noting why only sitemap is exempted (because this function is ported from Next.js where robots/manifest aren't relevant).

[Divkix]

Filed as follow-up issue #963. The inconsistency is theoretically latent since robots/manifest are nestable: false, but worth tracking.

[ask-bonk]

Just confirming: isParallelRouteSegment excludes @children (so it doesn't trigger hash suffixes), while getStaticMetadataRoute at line 390 uses seg.startsWith("@") (strips all @-prefixed segments including @children from the URL). The comment at 387-389 explains this well. The asymmetry is correct and intentional — @children is invisible in URLs but doesn't need disambiguation.

This is fine as-is; just flagging that the two checks look different for a reason.

[ask-bonk]

Nice — regex-matching the hash suffix rather than hardcoding a specific hash value is the right call. Makes the tests resilient to hash algorithm changes while still asserting the structural invariant.

[ask-bonk]

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

Overall: The PR correctly ports fillStaticMetadataSegment from Next.js. The core logic — - placeholders for dynamic segments, route group/parallel slot stripping, djb2 hash suffixes — is sound. All 69 tests pass.

Key comments posted:

1. Redundant ternary (line 398): lastSlash === -1 ? "" : "" — both branches return the same thing. Suggested collapsing to just "".

2. Duplicated logic / dead code (line 529): The inline IIFE in scanMetadataFiles duplicates what fillStaticMetadataSegment does but with config.urlPath instead of the actual filename. This means fillStaticMetadataSegment and getMetadataRouteFilename are exported/defined but never called in production code — only in tests. Suggested either refactoring to share the logic or documenting the intent.

3. Sitemap-only exemption inconsistency (line 399): getMetadataRouteSuffix only exempts sitemap from hash suffixes, while the existing metadataRouteSuffix also exempts robots and manifest. Inconsequential today but a latent inconsistency.

4. Positive note on @children handling (line 370): The asymmetry between isParallelRouteSegment (excludes @children) and getStaticMetadataRoute (strips all @ prefixes) is correct and well-commented.

5. Positive note on test approach (test line 608): Using regex matching for hash suffixes rather than hardcoded values is resilient and good practice.

github run

[Divkix]

All three review issues addressed:

1. Redundant ternary — collapsed in b8d8bb1
2. Duplicated logic — extracted computeMetadataRouteSuffix shared helper in b8d8bb1, now used by both scanMetadataFiles and fillStaticMetadataSegment/getMetadataRouteFilename
3. Sitemap-only exemption — filed as follow-up issue getMetadataRouteSuffix only exempts sitemap; metadataRouteSuffix also exempts robots and manifest #963

[james-elicx]

/bigbonk review

[ask-bonk]

All three issues from the previous review are addressed:

1. Redundant ternary — collapsed to lastSlash > 0 ? page.slice(0, lastSlash) : "".
2. Duplicated logic — extracted computeMetadataRouteSuffix as a shared helper used by both scanMetadataFiles (static branch) and fillStaticMetadataSegment/getMetadataRouteFilename. The core route+suffix computation is no longer duplicated.
3. Sitemap-only exemption — tracked in #963.

Code is clean, tests pass (69/69), vp check is green. The implementation correctly ports the Next.js fillStaticMetadataSegment behavior: dynamic segments become - placeholders, route groups and parallel slots are stripped from URLs, and djb2 hash suffixes disambiguate collisions from invisible parents.

One minor observation (not blocking): fillStaticMetadataSegment and getMetadataRouteFilename are still only exercised in tests — scanMetadataFiles uses computeMetadataRouteSuffix directly rather than delegating through fillStaticMetadataSegment. This is fine since the shared helper eliminates the drift risk, and fillStaticMetadataSegment is presumably intended for future runtime use. The dynamic branch in scanMetadataFiles still uses the old metadataRouteSuffix function, which produces identical hashes for the same input — verified by inspection.

[ask-bonk]

Review posted. All three issues from the previous review have been addressed:

1. Redundant ternary — fixed by collapsing to a simple conditional.
2. Duplicated logic — resolved via the shared computeMetadataRouteSuffix helper, now used by both scanMetadataFiles and fillStaticMetadataSegment.
3. Sitemap-only exemption — tracked as follow-up in getMetadataRouteSuffix only exempts sitemap; metadataRouteSuffix also exempts robots and manifest #963.

Tests pass (69/69), vp check is clean. Approved.

github run