Skip to content

fix: align metadataRouteSuffix exemption list with Next.js#969

Merged
james-elicx merged 1 commit intocloudflare:mainfrom
Divkix:fix/963-metadata-suffix-inconsistency
Apr 29, 2026
Merged

fix: align metadataRouteSuffix exemption list with Next.js#969
james-elicx merged 1 commit intocloudflare:mainfrom
Divkix:fix/963-metadata-suffix-inconsistency

Conversation

@Divkix
Copy link
Copy Markdown
Contributor

@Divkix Divkix commented Apr 29, 2026

Fixes #963

Summary

  • Removed extra robots and manifest exemptions from metadataRouteSuffix to match getMetadataRouteSuffix and Next.js
  • Both functions now only exempt sitemap from hash suffixes
  • The extra exemptions were latent (robots/manifest are root-only), but the inconsistency could cause bugs if nestable ever changes

Test plan

  • Existing tests/metadata-routes.test.ts (69 tests) pass
  • Existing tests/app-router.test.ts metadata tests pass

…e#963)

metadataRouteSuffix exempted robots and manifest in addition to sitemap,
while getMetadataRouteSuffix only exempts sitemap (matching Next.js).
This was latent because robots/manifest are nestable: false, but the
inconsistency could cause subtle bugs if nestable ever changes.

Remove the extra exemptions so both functions match Next.js exactly.
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 29, 2026

Open in StackBlitz

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

commit: c99f4ae

@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.

LGTM. The change is correct and the reasoning is sound.

The old robots and manifest exemptions in metadataRouteSuffix were dead code — both types have nestable: false in METADATA_FILE_MAP, so the scan loop at line 527 (if (!config.nestable && urlPrefix !== "") continue) already ensures they only appear at root with an empty parentSegments. An empty array means hasInvisibleParent is always false, so the function would have returned "" anyway before reaching the hash computation.

Removing the dead exemptions to match getMetadataRouteSuffix (which only exempts sitemap, per the Next.js port) is the right call. One minor comment on the updated wording.

if (metaType === "sitemap") {
// Sitemap is exempt per Next.js (robots/manifest are root-only, so
// invisible parents never apply — but we keep the exemption list
// matching getMetadataRouteSuffix for defensive consistency).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: the comment says "we keep the exemption list matching getMetadataRouteSuffix" — but the whole point of this PR is that we changed the exemption list to match. The comment reads as if we intentionally kept something unchanged. Consider rewording:

Suggested change
// matching getMetadataRouteSuffix for defensive consistency).
// matching getMetadataRouteSuffix and Next.js for consistency).

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 29, 2026

Review submitted. Here's a summary:

Approved with one minor comment suggestion.

The PR correctly removes dead robots and manifest exemptions from metadataRouteSuffix to align it with getMetadataRouteSuffix. The exemptions were unreachable because both types have nestable: false, meaning they only appear at root where parentSegments is always empty — so hasInvisibleParent would be false regardless, and the function would already return "" without hitting the exemption check.

Left one nit on the comment wording at line 467, suggesting a small tweak for clarity.

github run

@james-elicx james-elicx merged commit 3c19cf0 into cloudflare:main Apr 29, 2026
25 checks passed
@Divkix Divkix deleted the fix/963-metadata-suffix-inconsistency branch April 29, 2026 20:10
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.

getMetadataRouteSuffix only exempts sitemap; metadataRouteSuffix also exempts robots and manifest

2 participants