fix(vue): restore Style component inside Head and children prop#686
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe changes fix handling of VNode children text content in the Vue Head component by introducing a helper function to recursively extract plain text, normalizing props to use tag-specific keys ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Bundle Size Analysis
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/vue/test/unit/dom/headComponents.test.ts (1)
79-103: Exercise a multi-node children case here.
[css.value]only covers the one-item happy path. It won't fail whenextractTextContent()ignores sibling text nodes, which is the shape Vue often produces for adjacent interpolations or fragments. Add at least one case with two text entries or a fragment-wrapped text child.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue/test/unit/dom/headComponents.test.ts` around lines 79 - 103, Update the test "Style with dynamic slot content (array children)" to exercise a multi-node children shape (two adjacent text nodes or a fragment-wrapped text child) instead of only [css.value]; specifically, when building the Head slot in the App component (the render returning h(Head,...)), change the style children to include two text entries (e.g., [css.value, ' /* extra */'] or wrap with a Fragment that yields two text nodes) so resolveTags/ extractTextContent must concatenate siblings and the assertion on styleTag?.textContent still verifies the combined string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vue/src/components.ts`:
- Around line 11-18: The current extractTextContent logic only reads the first
array child and truncates multi-part text; update the Array.isArray(children)
branch in extractTextContent to iterate over all entries, map each entry through
the existing extraction logic (treat string entries as text, for objects with a
'children' property recurse via extractTextContent on VNode['children']),
collect the results and return the joined string (e.g., concatenated with ''),
ensuring other branches (string and object-with-children) remain unchanged.
In `@packages/vue/test/unit/dom/headComponents.test.ts`:
- Around line 12-25: The unused helper mountWithHead is causing CI failures and
uses CommonJS require; either remove the entire mountWithHead function or
convert it to a module-style helper and use it in tests: replace const {
createApp } = require('vue') with an ES import (import { createApp } from
'vue'), keep references to createHead and App as shown, ensure tests call
mountWithHead(renderFn) so it’s used; otherwise delete the mountWithHead
definition and any related unused symbols (createHead, App) from the file.
---
Nitpick comments:
In `@packages/vue/test/unit/dom/headComponents.test.ts`:
- Around line 79-103: Update the test "Style with dynamic slot content (array
children)" to exercise a multi-node children shape (two adjacent text nodes or a
fragment-wrapped text child) instead of only [css.value]; specifically, when
building the Head slot in the App component (the render returning h(Head,...)),
change the style children to include two text entries (e.g., [css.value, ' /*
extra */'] or wrap with a Fragment that yields two text nodes) so resolveTags/
extractTextContent must concatenate siblings and the assertion on
styleTag?.textContent still verifies the combined string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae5694b6-05a4-4f55-ad06-50e52a45e60f
📒 Files selected for processing (2)
packages/vue/src/components.tspackages/vue/test/unit/dom/headComponents.test.ts
harlan-zw
force-pushed
the
fix/517-vue-style-head-component
branch
from
44e8b5b to
ab501c3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vue/test/unit/dom/headComponents.test.ts`:
- Line 10: The file imports mount but never uses it, causing an ESLint error;
remove the unused import by deleting "mount" from the import statement (the
import line that currently reads "import { mount } from '../../util'") or
replace it with a used symbol from '../../util' if intended—ensure no other
references to mount exist in headComponents.test.ts and run lint/tests to
confirm.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bfd7612-77fd-4292-8c85-ce53bbeceed2
📒 Files selected for processing (2)
packages/vue/src/components.tspackages/vue/test/unit/dom/headComponents.test.ts
harlan-zw
force-pushed
the
fix/517-vue-style-head-component
branch
from
ab501c3 to
70ffaee
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/vue/test/unit/dom/headComponents.test.ts (1)
106-106: Drop unnecessaryawaitfor synchronousrenderDOMHead().
renderDOMHead()is synchronous here, so awaiting it adds noise and hides the intended API usage.Suggested cleanup
- await renderDOMHead(head, { document: dom.window.document }) + renderDOMHead(head, { document: dom.window.document })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue/test/unit/dom/headComponents.test.ts` at line 106, renderDOMHead is synchronous in this test, so remove the unnecessary "await" before the renderDOMHead(head, { document: dom.window.document }) call; replace the awaited invocation with a plain call to renderDOMHead(...) and, if the test function is only async because of that await, change the test to a non-async function (remove the async keyword) to reflect the synchronous API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/0.react/head/guides/1.core-concepts/2.reactivity.md`:
- Line 276: The ordered list item "Cleanup Happens Automatically: Unhead handles
cleanup when components unmount through React's effect cleanup system." is
incorrectly numbered as "1." after items 1–3; update its markdown list marker to
"4." (or the next sequential number used in the preceding list) so the
ordered-list numbering is consistent in the source.
In `@docs/head/1.guides/releases/v3.md`:
- Line 8: Replace the unhyphenated compound adjective "side-effect free" with
the hyphenated form "side-effect-free" in the sentence that describes the new
rendering engine; update the phrase in the paragraph containing "To fix this
properly, we had to make rendering synchronous, pluggable, and side-effect free"
so it reads "side-effect-free" for correct compound adjective usage and improved
readability.
---
Nitpick comments:
In `@packages/vue/test/unit/dom/headComponents.test.ts`:
- Line 106: renderDOMHead is synchronous in this test, so remove the unnecessary
"await" before the renderDOMHead(head, { document: dom.window.document }) call;
replace the awaited invocation with a plain call to renderDOMHead(...) and, if
the test function is only async because of that await, change the test to a
non-async function (remove the async keyword) to reflect the synchronous API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3959bee7-b6e2-43b7-9947-b4341d726cce
📒 Files selected for processing (47)
docs/0.angular/head/guides/0.get-started/1.installation.mddocs/0.angular/head/guides/0.get-started/2.migration.mddocs/0.angular/head/guides/1.core-concepts/0.reactivity.mddocs/0.angular/schema-org/guides/get-started/0.installation.mddocs/0.nuxt/head/guides/0.get-started/1.migration.mddocs/0.react/head/guides/0.get-started/1.installation.mddocs/0.react/head/guides/0.get-started/2.migration.mddocs/0.react/head/guides/0.get-started/migrate-from-react-helmet.mddocs/0.react/head/guides/1.core-concepts/2.reactivity.mddocs/0.react/schema-org/guides/get-started/0.installation.mddocs/0.solid-js/head/guides/0.get-started/1.installation.mddocs/0.solid-js/head/guides/0.get-started/2.migration.mddocs/0.solid-js/head/guides/1.core-concepts/0.reactivity.mddocs/0.solid-js/schema-org/guides/get-started/0.installation.mddocs/0.svelte/head/guides/0.get-started/1.installation.mddocs/0.svelte/head/guides/0.get-started/2.migration.mddocs/0.svelte/schema-org/guides/get-started/0.installation.mddocs/0.typescript/head/guides/0.get-started/1.installation.mddocs/0.typescript/head/guides/0.get-started/1.migration.mddocs/0.typescript/schema-org/guides/get-started/0.installation.mddocs/0.vue/head/guides/0.get-started/1.installation.mddocs/0.vue/head/guides/0.get-started/1.migration.mddocs/0.vue/schema-org/guides/0.get-started/0.installation.mddocs/head/1.guides/1.core-concepts/1.titles.mddocs/head/1.guides/1.core-concepts/2.positions.mddocs/head/1.guides/1.core-concepts/3.class-attr.mddocs/head/1.guides/1.core-concepts/4.inner-content.mddocs/head/1.guides/1.core-concepts/6.handling-duplicates.mddocs/head/1.guides/1.core-concepts/8.dom-event-handling.mddocs/head/1.guides/1.core-concepts/9.loading-scripts.mddocs/head/1.guides/2.advanced/11.extending-unhead.mddocs/head/1.guides/2.advanced/7.client-only-tags.mddocs/head/1.guides/plugins/6.template-params.mddocs/head/1.guides/plugins/alias-sorting.mddocs/head/1.guides/plugins/canonical.mddocs/head/1.guides/plugins/infer-seo-meta-tags.mddocs/head/1.guides/plugins/validate.mddocs/head/1.guides/releases/v3.mddocs/head/7.api/composables/0.use-head.mddocs/head/7.api/composables/4.use-script.mddocs/schema-org/2.guides/0.get-started/0.overview.mddocs/schema-org/2.guides/4.recipes/0.custom-nodes.mddocs/schema-org/2.guides/4.recipes/blog.mddocs/schema-org/2.guides/4.recipes/faq.mddocs/schema-org/5.api/0.composables/0.use-schema-org.mdpackages/vue/src/components.tspackages/vue/test/unit/dom/headComponents.test.ts
✅ Files skipped from review due to trivial changes (44)
- docs/0.svelte/schema-org/guides/get-started/0.installation.md
- docs/0.solid-js/head/guides/0.get-started/1.installation.md
- docs/0.solid-js/schema-org/guides/get-started/0.installation.md
- docs/0.react/schema-org/guides/get-started/0.installation.md
- docs/0.angular/schema-org/guides/get-started/0.installation.md
- docs/0.vue/schema-org/guides/0.get-started/0.installation.md
- docs/head/1.guides/1.core-concepts/9.loading-scripts.md
- docs/0.react/head/guides/0.get-started/2.migration.md
- docs/0.angular/head/guides/0.get-started/2.migration.md
- docs/0.typescript/schema-org/guides/get-started/0.installation.md
- docs/head/1.guides/1.core-concepts/8.dom-event-handling.md
- docs/0.typescript/head/guides/0.get-started/1.installation.md
- docs/0.vue/head/guides/0.get-started/1.installation.md
- docs/0.svelte/head/guides/0.get-started/1.installation.md
- docs/head/1.guides/1.core-concepts/3.class-attr.md
- docs/head/7.api/composables/4.use-script.md
- docs/0.solid-js/head/guides/0.get-started/2.migration.md
- docs/0.react/head/guides/0.get-started/1.installation.md
- docs/head/1.guides/1.core-concepts/4.inner-content.md
- docs/0.angular/head/guides/0.get-started/1.installation.md
- docs/head/1.guides/1.core-concepts/6.handling-duplicates.md
- docs/schema-org/2.guides/0.get-started/0.overview.md
- docs/head/1.guides/2.advanced/7.client-only-tags.md
- docs/0.react/head/guides/0.get-started/migrate-from-react-helmet.md
- docs/head/1.guides/plugins/6.template-params.md
- docs/0.angular/head/guides/1.core-concepts/0.reactivity.md
- docs/schema-org/2.guides/4.recipes/0.custom-nodes.md
- docs/0.nuxt/head/guides/0.get-started/1.migration.md
- docs/schema-org/2.guides/4.recipes/faq.md
- docs/head/1.guides/plugins/alias-sorting.md
- docs/0.vue/head/guides/0.get-started/1.migration.md
- docs/schema-org/2.guides/4.recipes/blog.md
- docs/head/1.guides/plugins/canonical.md
- docs/0.svelte/head/guides/0.get-started/2.migration.md
- docs/head/1.guides/1.core-concepts/1.titles.md
- docs/head/7.api/composables/0.use-head.md
- docs/schema-org/5.api/0.composables/0.use-schema-org.md
- docs/head/1.guides/plugins/validate.md
- docs/0.solid-js/head/guides/1.core-concepts/0.reactivity.md
- docs/head/1.guides/2.advanced/11.extending-unhead.md
- docs/head/1.guides/plugins/infer-seo-meta-tags.md
- docs/0.typescript/head/guides/0.get-started/1.migration.md
- packages/vue/src/components.ts
- docs/head/1.guides/1.core-concepts/2.positions.md
| ``` | ||
|
|
||
| 4. **Cleanup Happens Automatically**: Unhead handles cleanup when components unmount through React's effect cleanup system. | ||
| 1. **Cleanup Happens Automatically**: Unhead handles cleanup when components unmount through React's effect cleanup system. |
There was a problem hiding this comment.
Keep ordered-list numbering consistent in source markdown.
This item is written as 1. after items 1-3, which makes the source inconsistent and can render unexpectedly depending on parser/settings.
Proposed fix
-1. **Cleanup Happens Automatically**: Unhead handles cleanup when components unmount through React's effect cleanup system.
+4. **Cleanup Happens Automatically**: Unhead handles cleanup when components unmount through React's effect cleanup system.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. **Cleanup Happens Automatically**: Unhead handles cleanup when components unmount through React's effect cleanup system. | |
| 4. **Cleanup Happens Automatically**: Unhead handles cleanup when components unmount through React's effect cleanup system. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/0.react/head/guides/1.core-concepts/2.reactivity.md` at line 276, The
ordered list item "Cleanup Happens Automatically: Unhead handles cleanup when
components unmount through React's effect cleanup system." is incorrectly
numbered as "1." after items 1–3; update its markdown list marker to "4." (or
the next sequential number used in the preceding list) so the ordered-list
numbering is consistent in the source.
| title: "v3" | ||
| --- | ||
|
|
||
| Unhead v3 rebuilds the rendering engine from the ground up. The motivation: **streaming SSR**. Frameworks like Nuxt, SolidStart, and SvelteKit stream HTML to the browser as data loads, but head tags were still stuck in a request/response model, resolved once and never updated. To fix this properly, we had to make rendering synchronous, pluggable, and side-effect free. The result is a faster, smaller, and more capable head manager. |
There was a problem hiding this comment.
Hyphenate the compound adjective for readability.
Use side-effect-free instead of side-effect free in this sentence.
🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: Use a hyphen to join words.
Context: ... synchronous, pluggable, and side-effect free. The result is a faster, smaller, a...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/head/1.guides/releases/v3.md` at line 8, Replace the unhyphenated
compound adjective "side-effect free" with the hyphenated form
"side-effect-free" in the sentence that describes the new rendering engine;
update the phrase in the paragraph containing "To fix this properly, we had to
make rendering synchronous, pluggable, and side-effect free" so it reads
"side-effect-free" for correct compound adjective usage and improved
readability.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
harlan-zw
force-pushed
the
fix/517-vue-style-head-component
branch
from
8bd589d to
954e680
Compare
This was referenced Apr 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
Resolves #517
❓ Type of change
📚 Description
addVNodeToHeadObjhad two bugs introduced in v2: it readnode.children[0]['children']which was off by one indirection for text VNodes, and stored extracted content asprops.childrenwhich isn't a recognized key in unhead v3. Fixed by addingextractTextContent()helper that handles all VNode child shapes, and mapping the content totextContent(for style/noscript/title) orinnerHTML(for script). Also restores backward-compatchildrenprop support.Summary by CodeRabbit
Release Notes
Bug Fixes
Tests