-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
feat(kit): add setGlobalHead utility
#33512
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
Conversation
|
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
WalkthroughA new utility Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🪛 LanguageTooldocs/3.api/5.kit/9.head.md[uncategorized] ~16-~16: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) [uncategorized] ~18-~18: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) [uncategorized] ~44-~44: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) [uncategorized] ~48-~48: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/kit/src/head.ts (1)
5-8: Document semantics and add light guards
- Add JSDoc stating precedence rules (who wins for objects and arrays) and that this must run within a Nuxt context (
useNuxt()).- Consider accepting partial input:
addAppHead(head: Partial<NuxtAppConfig['head']>).- No‑op on falsy input to avoid accidental writes:
export function addAppHead (head: NuxtAppConfig['head']) { const nuxt = useNuxt() - nuxt.options.app.head = defu(head, nuxt.options.app.head) + if (!head) return + nuxt.options.app.head = defu(nuxt.options.app.head, head) }Also add a test covering object fields (e.g.
titleTemplate) and array fields (e.g. duplicatemeta[name="viewport"]) to lock in the intended precedence. Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/kit/src/head.ts(1 hunks)packages/kit/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/kit/src/index.tspackages/kit/src/head.ts
🧬 Code graph analysis (1)
packages/kit/src/head.ts (1)
packages/kit/src/index.ts (2)
addAppHead(19-19)useNuxt(29-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: code
🔇 Additional comments (1)
packages/kit/src/index.ts (1)
19-19: Public export looks goodExport wiring is correct and consistent with other Kit utils. No concerns.
packages/kit/src/head.ts
Outdated
| export function addAppHead (head: NuxtAppConfig['head']) { | ||
| const nuxt = useNuxt() | ||
| nuxt.options.app.head = defu(head, nuxt.options.app.head) | ||
| } |
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.
🧩 Analysis chain
Merge precedence likely reversed; modules override user config
Using defu(head, nuxt.options.app.head) makes module-provided values win for object keys (e.g. title, htmlAttrs). Typically, user config should take precedence and modules should provide defaults. Flipping the order also avoids mutating the caller’s head object (defu mutates its first argument). Apply:
- nuxt.options.app.head = defu(head, nuxt.options.app.head)
+ nuxt.options.app.head = defu(nuxt.options.app.head, head)Caveat: defu v6 concatenates arrays as value + defaults. After flipping, arrays become user + module; if Unhead dedupes with “last wins”, this may give modules precedence for duplicate tags. Please confirm intended behaviour and adjust with a custom array merge (e.g. defuArrayFn) if user‑last should win for arrays. Based on learnings.
I'll verify the key claims in this review comment by researching the defu function behaviour and how it handles merge precedence and arrays.
Now I need to verify the Unhead deduplication semantics to complete the analysis.
Now I need to verify the context of `addAppHead` in Nuxt to understand the intended behaviour.
Based on my verification of the technical details:
The review comment is fundamentally correct; no changes needed.
Leftmost arguments have more priority when assigning defaults in defu, which means defu(head, nuxt.options.app.head) gives the module-provided head precedence over nuxt.options.app.head (user config). This is backwards—user configuration should typically take precedence.
The proposed fix (defu(nuxt.options.app.head, head)) correctly reverses this order. The caveat about arrays is also accurate: defu will concat array values such that defu({ array: ["b", "c"] }, { array: ["a"] }) results in { array: ['b', 'c', 'a'] }, and Unhead's default deduplication strategy is "most recent wins". After flipping the arguments, arrays become [user + module] in defu's concatenation order, which could affect which tag wins in Unhead's deduplication logic if both user and module provide duplicate tags.
The review comment appropriately flags this interaction and asks for confirmation of intended behaviour.
🌐 Web query:
defu function merge precedence which argument wins
💡 Result:
defu gives precedence to the leftmost arguments — earlier (left) args override later/default args. Example: defu({a:{b:2}}, {a:{b:1,c:3}}) -> {a:{b:2,c:3}}. [1][2]
🌐 Web query:
Nuxt addAppHead function user config modules precedence
💡 Result:
Short answer — user app.head (the nuxt.config app.head / app.config) takes precedence if modules merge correctly; modules run in the module order (items in nuxt.config first, then modules/ directory alphabetical) and should merge their defaults into nuxt.options.app.head (typically via defu) so user config overrides module-provided head. Cite: Nuxt app head docs and modules + example of modules merging with defu.
Sources:
[1] Nuxt — App Head / SEO & Meta.
[2] Nuxt — Modules execution order (modules run sequentially; nuxt.config modules first, then modules/ directory alphabetical).
[3] Nuxt Kit example showing modules merging head/defaults with defu (module should extend nuxt.options.app.head so user config overrides).
Apply the suggested fix to correct reversed merge precedence; confirm array deduplication behaviour with Unhead.
The review comment is correct. Defu gives precedence to the leftmost arguments, so defu(head, nuxt.options.app.head) incorrectly makes module-provided values win. Nuxt design requires user app.head to take precedence, with modules merging their defaults via defu so that user config overrides module-provided head.
Flipping the argument order to defu(nuxt.options.app.head, head) is the correct fix. However, the caveat about arrays is valid: after the flip, arrays will be concatenated as [existing + module], placing module values last. If Unhead's deduplication uses "most recent (last) wins" semantics for duplicate tags, module values may still take precedence for duplicated array items. Confirm with Unhead documentation and adjust with a custom array merge function (e.g. defuArrayFn) if user-provided array values should consistently win.
🤖 Prompt for AI Agents
In packages/kit/src/head.ts around lines 5 to 8, the defu call currently
reverses merge precedence (module head is winning); change to call defu with the
existing app head first and module head second so user app.head wins (i.e.
defu(nuxt.options.app.head, head)); after that, verify Unhead’s array
deduplication semantics (it may treat last-occurring tags as authoritative), and
if user-provided array entries must always override module entries implement a
custom array-merge function (e.g. defuArrayFn) to deduplicate/merge arrays so
user values retain precedence.
CodSpeed Performance ReportMerging #33512 will improve performances by 12.17%Comparing Summary
Benchmarks breakdown
Footnotes |
addAppHead utilitysetDefaultHead utility
|
Lgmt there's probably some improvement to be made on deduping as it's quite possible module authors could introduce tags that conflict with end users, but we can always improve that later |
setDefaultHead utilitysetGlobalHead utility
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/kit/src/head.ts (1)
5-8: Critical: Reversed merge precedence must be fixed.The merge order
defu(head, nuxt.options.app.head)gives module-provided values precedence over user configuration, which contradicts Nuxt's design principle that userapp.headconfiguration should always override module defaults. This was flagged in a previous review but remains unfixed.Apply this diff to correct the merge precedence:
- nuxt.options.app.head = defu(head, nuxt.options.app.head) + nuxt.options.app.head = defu(nuxt.options.app.head, head)Note: After this fix, the documentation statement "your provided values taking precedence" (line 15 of docs/3.api/5.kit/9.head.md) will need clarification, as it will then mean "existing user config takes precedence over module-provided defaults", which is the correct behaviour. The current documentation is misleading about the intended semantics. Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/3.api/5.kit/9.head.md(1 hunks)packages/kit/src/head.ts(1 hunks)packages/kit/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/kit/src/head.tspackages/kit/src/index.ts
🧬 Code graph analysis (1)
packages/kit/src/head.ts (1)
packages/kit/src/index.ts (2)
setGlobalHead(19-19)useNuxt(29-29)
🪛 LanguageTool
docs/3.api/5.kit/9.head.md
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...our provided values taking precedence. ::tip This is particularly useful for mod...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~18-~18: Loose punctuation mark.
Context: ..., or scripts into the application head. :: ### Type ```ts twoslash // @errors: ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...ith existing configuration: - charset: Character encoding for the document - `...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~48-~48: Loose punctuation mark.
Context: ...cts (stylesheets, icons, etc.) - style: Array of inline style tag objects - `sc...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: codeql (actions)
- GitHub Check: code
🔇 Additional comments (3)
packages/kit/src/index.ts (1)
19-19: LGTM!The export is correctly placed in the Utils section and follows the established pattern for Nuxt Kit utilities.
packages/kit/src/head.ts (1)
1-3: LGTM!All imports are necessary and correctly structured for the function implementation.
docs/3.api/5.kit/9.head.md (1)
56-132: LGTM!The usage examples are comprehensive, well-structured, and demonstrate common use cases effectively. The code examples follow best practices for Nuxt modules.
| ## `setGlobalHead` | ||
|
|
||
| Sets global head configuration for your Nuxt application. This utility allows modules to programmatically configure meta tags, links, scripts, and other head elements that will be applied across all pages. | ||
|
|
||
| The provided head configuration will be merged with any existing head configuration using deep merging, with your provided values taking precedence. | ||
|
|
||
| ::tip | ||
| This is particularly useful for modules that need to inject global meta tags, stylesheets, or scripts into the application head. | ||
| :: |
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.
Clarify merge precedence semantics in documentation.
Line 15 states "with your provided values taking precedence", which is ambiguous. After the critical merge precedence fix in packages/kit/src/head.ts is applied, this should be reworded to clearly communicate that setGlobalHead sets default head configuration that user app.head configuration can override.
Consider revising to:
The provided head configuration will be merged with any existing head configuration using deep merging, with existing user configuration taking precedence over module-provided defaults.This clarifies the correct behaviour: modules set defaults, users override.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...our provided values taking precedence. ::tip This is particularly useful for mod...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~18-~18: Loose punctuation mark.
Context: ..., or scripts into the application head. :: ### Type ```ts twoslash // @errors: ...
(UNLIKELY_OPENING_PUNCTUATION)
🤖 Prompt for AI Agents
In docs/3.api/5.kit/9.head.md around lines 11 to 19, the sentence "with your
provided values taking precedence" is misleading after the head merge precedence
fix; update the wording to state that module-provided head values act as
defaults and are deep-merged with existing user/app head configuration, with the
existing user/app configuration taking precedence (i.e., user app.head overrides
module defaults). Replace the ambiguous line with a clear sentence such as: "The
provided head configuration will be deep-merged with any existing head
configuration; existing user/app configuration takes precedence over
module-provided defaults."
🔗 Linked issue
resolve #25469
📚 Description
no response