fix(vue): keep reactive input at useHead boundary - #886
Conversation
VueHeadClient accepts reactive UseHeadInput values, but raw client entries neither resolved nor watched top-level refs and getters. Scope each watcher to its returned entry so patching replaces the source and disposal stops updates.
📝 WalkthroughWalkthroughVue head client and streaming APIs now use aligned ChangesVue head input contracts
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 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 |
📦 Bundle Size✅ No notable changes All bundles (14)
📦 Runtime Dependencies✅ No runtime dependency changes All packages (10)
Skipped optional dependencies (114)
Production dependencies only. Peer dependencies and Unhead workspace packages are excluded. Skipped optional dependencies are unavailable on the CI platform. ⚡ Performance (directional)✅ No significant change (within CI noise) All benchmarks (25)
Baseline: main @ 6b2c5b9 · 2026-07-29 · gzipped is the headline size metric · perf is directional (shared-runner, gated) |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/vue/test/unit/dom/reactivePush.test.ts (2)
14-21: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDispose the entry created by the first test.
The returned entry is discarded, so its
watchEffectremains active after the assertions. Retain it and calldispose()during cleanup; Vue documents the returned watcher handle as the mechanism for stopping the effect. (vuejs.org)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vue/test/unit/dom/reactivePush.test.ts` around lines 14 - 21, Retain the entry returned by renderDOMHead in the first test and call its dispose() method during test cleanup after the assertions. Update the reactivePush test setup without changing the existing title update expectations.
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the raw getter branch.
The implementation has a separate
typeof input === 'function'path, but the tests only passcomputed(...). Add a case usinghead.push(() => ({ title: title.value }))so raw getter reactivity and disposal are protected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vue/test/unit/dom/reactivePush.test.ts` at line 14, Add a test case in the reactive push tests that calls head.push with a raw getter function returning the title object, rather than computed(...). Verify the getter remains reactive and is properly disposed, covering the implementation’s typeof input === 'function' branch while preserving the existing computed case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/vue/test/unit/dom/reactivePush.test.ts`:
- Around line 14-21: Retain the entry returned by renderDOMHead in the first
test and call its dispose() method during test cleanup after the assertions.
Update the reactivePush test setup without changing the existing title update
expectations.
- Line 14: Add a test case in the reactive push tests that calls head.push with
a raw getter function returning the title object, rather than computed(...).
Verify the getter remains reactive and is properly disposed, covering the
implementation’s typeof input === 'function' branch while preserving the
existing computed case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e9f1598-6e75-4a0d-ab79-b02d16f259b5
📒 Files selected for processing (2)
packages/vue/src/client.tspackages/vue/test/unit/dom/reactivePush.test.ts
|
Addressed both review test gaps in 2f9ab79: the computed-input test now disposes its entry, and a direct getter test covers updates plus watcher cleanup. Targeted 4/4 tests, ESLint, and diff check pass. |
The v2 core rewrite removed client-side Vue input resolution while preserving reactive VueHeadClient input types. Resolve Vue values during rendering without adding watcher ownership to raw entries.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/vue/test/unit/dom/pushResolver.test.ts (1)
40-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover raw getter lifecycle cleanup.
This only tests a computed ref and discards the returned entry. Add a direct
head.push(() => …)case that verifies updates, then patch/dispose the entry and confirms it no longer triggers evaluation/rendering. This is required by the PR objective’s watcher-cleanup contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vue/test/unit/dom/pushResolver.test.ts` around lines 40 - 63, Add a complementary test near the existing computed-input test that pushes a raw getter via head.push(() => …), verifies its initial and updated evaluation/rendering behavior, then patches or disposes the returned entry and confirms subsequent changes no longer evaluate the getter or update the DOM, covering the watcher-cleanup contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/vue/test/unit/dom/pushResolver.test.ts`:
- Around line 40-63: Add a complementary test near the existing computed-input
test that pushes a raw getter via head.push(() => …), verifies its initial and
updated evaluation/rendering behavior, then patches or disposes the returned
entry and confirms subsequent changes no longer evaluate the getter or update
the DOM, covering the watcher-cleanup contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5710fc6b-75fb-4958-ad67-7eb7eaafbee5
📒 Files selected for processing (2)
packages/vue/src/client.tspackages/vue/test/unit/dom/pushResolver.test.ts
|
Skipping the raw getter lifecycle suggestion. This revision deliberately removes watcher ownership from |
🤖 Harlan Agent Kit automated reviewThis was generated and posted by a robot, not Harlan's personal review. PASS · 95/100 confidence
Human merge decision still required. |
…-push # Conflicts: # packages/vue/test/unit/types.test.ts
🔗 Linked issue
No linked issue.
❓ Type of change
📚 Description
Client head factories now accept core
ResolvableHeadinput. Vue refs and computed values remain supported throughuseHead()and by server head factories.This keeps Vue unwrapping inside the composable that owns dependency tracking and lifecycle cleanup. It also avoids adding a Vue resolver walk to every raw client entry.
Raw client
head.push()calls no longer accept Vue refs or computed values.📝 Migration
const title = ref('Reactive title') -head.push({ title }) +useHead({ title }, { head })Summary by CodeRabbit
Bug Fixes
Documentation
useHead()for reactive tracking and cleanup.Tests