fix(metadata): omit unused parent arg for cached generateMetadata

[james-elicx]

Problem

On routes where a parent layout uses a module-level 'use cache' directive and exports a generateMetadata() that takes no arguments, vinext logged:

Only plain objects can be passed to Server Functions from the Client. URL objects are not supported.
  {metadataBase: URL, description: ..., openGraph: ..., twitter: ..., title: ...}
                 ^^^

(Reproduced on app-router-playground at GET /layouts/clothing.)

Root cause

'use cache' wraps generateMetadata in the cache runtime. vinext always invoked it as generateMetadata(props, parent), where parent resolves to the merged ancestor metadata. When an ancestor sets metadataBase: new URL(...), the cache-key encoder (encodeReply) awaited the parent promise and tried to serialize the URL instance — which React rejects.

Fix (matches Next.js)

Next.js omits the parent argument for generateMetadata functions that don't use it (resolve-metadata.ts getResult / useCacheFunctionInfo.usedArgs[1]), precisely so non-serializable parent values never reach the cache encoder.

• resolveModuleMetadata now only passes parent when generateMetadata declares it (fn.length >= 2).
• registerCachedFunction preserves the original function's arity on the wrapper (the wrapper is (...args) / arity 0, which otherwise hides the signature) so the arity check works for cached functions too.

Verification

• Reproduced the warning, then confirmed it is gone after the fix; metadataBase URL resolution still works (og:image → absolute URL).
• Added regression tests in tests/app-page-head.test.ts (parent omitted for arity-0, still passed for arity-2, metadataBase URL preserved) and tests/shims.test.ts (arity-preserving wrapper).
• features, file-based-metadata, nextjs-compat/metadata, and shims suites pass (1395 tests); vp check is clean.

[james-elicx]

/bigbonk review

[pkg-pr-new]

Open in StackBlitz

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

commit: 13ad380

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
web	preview	production

[ask-bonk]

Review: fix(metadata): omit unused parent arg for cached generateMetadata

Solid, well-targeted fix. The two changes are coherent and reinforce each other: resolveModuleMetadata gates the parent arg on arity (fn.length >= 2), and registerCachedFunction preserves the wrapped function's arity so the gate works for 'use cache' exports too. I confirmed the end-to-end path — the Vite transform (index.ts:3939) replaces mod.generateMetadata with the registerCachedFunction wrapper, so without the Object.defineProperty arity restore the wrapper would report length === 0 and parent would always be omitted. The two pieces are necessary together.

Verified:

• tests/app-page-head.test.ts + tests/shims.test.ts pass (1032 tests).
• Ancestor metadata accumulation is unaffected: accumulatedMetadata in server/app-page-head.ts:308-314 merges from metadataResults, independent of whether a child consumes parent. So omitting parent for an arity-0 child does not break inherited metadataBase (the test confirms metadataBase URL still flows through).
• generateViewport does not pass a parent arg (metadata.tsx:44), so it cannot hit the same URL serialization bug — no parity gap introduced there.

Minor caveat (non-blocking):
The fn.length >= 2 heuristic approximates Next.js's static usage analysis (usedArgs[1]), and JS arity has edge cases where it diverges:

• generateMetadata(props, parent = fallback) reports length === 1 → vinext would skip parent even though it's declared.
• generateMetadata(props = {}, parent) reports length === 0.

These are uncommon (a default value on the parent param of generateMetadata is unusual), and the PR comment already notes Next.js uses usedArgs[1] rather than arity. The approximation is reasonable; just worth a one-line note in the code comment that arity is an approximation of usage, in case someone later hits a default-param case.

Nit:
The try/catch around Object.defineProperty for the non-configurable length case is defensive but effectively dead — function length is always configurable: true per spec, and the wrapper is a fresh closure. Harmless, but it could be a plain Object.defineProperty call. Not worth changing.

Overall this matches Next.js behavior, has good regression coverage, and the dev/prod parity concern (the cached-wrapper path) is handled. LGTM.

[ask-bonk]

Review posted to PR #1719.

Summary

The PR is a clean, well-targeted fix. I verified the full mechanism end-to-end:

The fix works as designed:

• The Vite transform (index.ts:3939) replaces mod.generateMetadata with the registerCachedFunction wrapper, which is declared (...args) → arity 0. The new Object.defineProperty arity restore is what makes the fn.length >= 2 gate in resolveModuleMetadata actually function for 'use cache' exports. The two changes are necessary together.
• Ancestor metadata accumulation is unaffected — accumulatedMetadata merges from metadataResults independent of whether the child consumes parent, so inherited metadataBase still flows through (confirmed by the test).
• generateViewport doesn't pass parent, so no parallel bug exists there.

Tests pass: 1032 tests across the two changed test files.

Minor caveat I flagged (non-blocking): fn.length >= 2 approximates Next.js's static usage analysis (usedArgs[1]). It diverges on default-param edge cases like generateMetadata(props, parent = x) (reports length === 1). Uncommon and acceptable, but worth a code-comment note that arity approximates usage.

Nit: the try/catch around Object.defineProperty is effectively dead since function length is always configurable per spec. Harmless.

Overall: matches Next.js behavior, good regression coverage, dev/prod parity handled. LGTM.

github run

[james-elicx]

/bigbonk re-review for issue and parity

[james-elicx]

/bigbonk re-review for issues and parity

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[ask-bonk]

Re-review: fix(metadata): omit unused parent arg for cached generateMetadata

Re-reviewed after the two prior-review nits were addressed (arity-approximation note added to the code comment, dead try/catch dropped). The PR is in good shape — verified end-to-end and confirmed no parity gaps were introduced.

Verified

• Both changes are coherent and jointly necessary. resolveModuleMetadata gates parent on mod.generateMetadata.length >= 2 (metadata.tsx:560-561), and registerCachedFunction restores the wrapped function's arity (cache-runtime.ts:582) so the gate works for 'use cache' exports. Without the arity restore the (...args) wrapper reports length === 0 and parent would always be omitted.
• Mechanism confirmed. The cache wrapper feeds args through encodeReply for key generation (cache-runtime.ts:447). Passing the resolved parent metadata (which can carry a URL metadataBase) is what produced the "URL objects are not supported" log. Omitting parent for arity-0/1 functions avoids that path entirely.
• Ancestor accumulation is unaffected. accumulatedMetadata in server/app-page-head.ts:236-244 / 308-314 merges from metadataResults independently of whether a child consumes parent, so inherited metadataBase still flows through. The new app-page-head.test.ts test confirms metadataBase URL is preserved through an arity-0 child.
• No parallel parity bug. generateViewport (metadata.tsx:44) and generateImageMetadata (server/file-based-metadata.ts:505, server/metadata-route-response.ts:285) only ever receive a single { params } arg — none pass parent, so none can hit the same serialization issue.
• Narrow blast radius. The wrapper length override is consumed only by resolveModuleMetadata; hasGenerateMetadata (app-page-head.ts:143) checks existence, not arity, and no other code reads cached-function arity.
• Tests pass: 1032 tests across tests/app-page-head.test.ts + tests/shims.test.ts. vp check clean on all four changed files.

Non-blocking

• The fn.length >= 2 heuristic approximates Next.js's static usage analysis (usedArgs[1]), diverging only on default-param signatures like (props, parent = x). This is now documented in the code comment (metadata.tsx:556-559) and is acceptable.

Matches Next.js behavior, good regression coverage, dev/prod parity handled (the cached-wrapper path is the shared mechanism). LGTM.

[ask-bonk]

Review posted to PR #1719.

Summary

Re-reviewed PR #1719 after the prior nits were addressed (arity-approximation note added, dead try/catch removed). Verified the fix end-to-end and found no issues or parity gaps.

The fix is correct and complete:

• The two changes (resolveModuleMetadata arity gate + registerCachedFunction arity restore) are jointly necessary — without the Object.defineProperty on cache-runtime.ts:582, the (...args) wrapper would report length === 0 and parent would always be dropped.
• Confirmed the bug mechanism: cached functions feed args through encodeReply for cache-key generation, and the resolved parent metadata can carry a URL metadataBase, triggering the "URL objects are not supported" log. Omitting parent for arity-0/1 functions avoids that.
• Ancestor metadata accumulation is unaffected — metadataBase still flows through to arity-0 children (test-confirmed).

No parity gaps introduced: generateViewport and generateImageMetadata only ever receive a single { params } arg, so they can't hit the same issue. The wrapper length override is consumed only by resolveModuleMetadata.

Tests: 1032 pass across the two changed test files; vp check clean on all four files.

Non-blocking: The fn.length >= 2 heuristic approximates Next.js's usedArgs[1] static analysis (diverges only on uncommon default-param signatures) — now properly documented in the code comment.

LGTM.

github run