fix(legacy): restore exports for v2 migration#741
Merged
Conversation
…tion The legacy entry points (`unhead/legacy` and `@unhead/vue/legacy`) regressed to a deprecation warning, breaking v2 consumers mid-migration. Re-export `createHead`, `createServerHead`, `getActiveHead`, and `createHeadCore` with the `DeprecationsPlugin` (plus the other legacy plugins for the core entry) pre-registered so v2 tag props keep working.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a legacy entrypoint and exports for Unhead and Vue: build config and package exports for Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant LegacyAPI as Legacy Entrypoint
participant UnheadFactory as Unhead Factory
participant Hooks as Entry Hooks
participant Deprecator as DeprecationsPlugin
App->>LegacyAPI: createHead(options)
LegacyAPI->>UnheadFactory: create head with merged legacyPlugins
UnheadFactory->>UnheadFactory: initialize instance (assign activeHead)
App->>UnheadFactory: push entries/tags
UnheadFactory->>Hooks: emit entries:normalize
Hooks->>Deprecator: invoke for each tag
Deprecator->>Deprecator: map children→innerHTML, hid/vmid→key, body→tagPosition, renderPriority→tagPriority
Deprecator->>UnheadFactory: return normalized entries
UnheadFactory->>App: render/manage normalized head entries
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 3
🤖 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/unhead/src/legacy/index.ts`:
- Line 57: The function createServerHead currently uses
Omit<CreateServerHeadOptions, 'propsResolver'> which is a typo and fails to
exclude the actual internal key; update the Omit to exclude 'propResolvers'
instead so propResolvers is not exposed to callers — locate the
createServerHead<T ...> declaration and replace the Omit key from
'propsResolver' to 'propResolvers' to enforce the intended API contract
referencing CreateServerHeadOptions and the propResolvers property.
- Around line 21-35: The current remapping uses truthy checks and thus skips
falsy-but-valid values (e.g., empty string) so legacy props like children, hid,
vmid, and body can leak; update the checks to key-existence checks (e.g., use
Object.prototype.hasOwnProperty.call(tag.props, 'children') or the 'in'
operator) for tag.props.children -> tag.innerHTML, tag.props.hid -> tag.key,
tag.props.vmid -> tag.key, and tag.props.body -> tag.tagPosition, and still
delete the original prop after remapping (keep using tag.innerHTML, tag.key,
tag.tagPosition as the target symbols).
In `@packages/vue/src/legacy.ts`:
- Around line 15-19: The legacy Vue wrapper createHead currently only prepends
DeprecationsPlugin when calling _createClientHead, but must include the full v2
compatibility plugin set used by the core legacy entry; update the plugins value
passed to _createClientHead so it prepends the same compatibility plugins array
as the core legacy entry (not just DeprecationsPlugin), e.g. replace the single
DeprecationsPlugin prepended in createHead with the full v2-compat plugins list
used in the core legacy implementation, and do the same change for the other
legacy wrapper function on lines 26-30 so both createHead wrappers mirror the
core legacy plugin set.
🪄 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: 4d224cfe-9cbe-4828-aa83-26a2c1ec74c4
📒 Files selected for processing (4)
packages/unhead/build.config.tspackages/unhead/package.jsonpackages/unhead/src/legacy/index.tspackages/vue/src/legacy.ts
harlan-zw
changed the title
- @unhead/vue/legacy createHead/createServerHead now prepend the full v2 migration plugin set (DeprecationsPlugin, PromisesPlugin, TemplateParamsPlugin, AliasSortingPlugin) to mirror unhead/legacy - Fix Omit<CreateServerHeadOptions, 'propsResolver'> typo → 'propResolvers'
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/unhead/src/legacy/index.ts (1)
21-35:⚠️ Potential issue | 🟡 MinorUse key-existence checks when remapping legacy props.
Line 21 / Line 25 / Line 29 / Line 33 currently skip falsy-but-valid values, so
children: '',hid: '',vmid: '', andbody: falsecan leak through without normalization or cleanup.🔧 Proposed fix
- if (tag.props.children) { + if ('children' in tag.props) { tag.innerHTML = tag.props.children delete tag.props.children } - if (tag.props.hid) { + if ('hid' in tag.props) { tag.key = tag.props.hid delete tag.props.hid } - if (tag.props.vmid) { + if ('vmid' in tag.props) { tag.key = tag.props.vmid delete tag.props.vmid } - if (tag.props.body) { - tag.tagPosition = 'bodyClose' - delete tag.props.body - } + if ('body' in tag.props) { + if (tag.props.body) + tag.tagPosition = 'bodyClose' + delete tag.props.body + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/unhead/src/legacy/index.ts` around lines 21 - 35, The current remapping logic uses truthy checks and therefore skips valid falsy values; change each conditional to check property existence (e.g., Object.prototype.hasOwnProperty.call(tag.props, 'children') or 'children' in tag.props) before remapping so empty strings and boolean false are handled and then perform the same assignment and delete (apply this for children -> innerHTML, hid -> key, vmid -> key, and body -> tagPosition).
🤖 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/legacy.ts`:
- Line 15: The exported legacyPlugins constant currently lacks a public type
annotation causing declaration emit errors (TS2883); add an explicit type
annotation so its exported type is stable: annotate legacyPlugins as
NonNullable<CreateClientHeadOptions['plugins']> (i.e. HeadPluginInput[]) and
ensure the export remains export const legacyPlugins:
NonNullable<CreateClientHeadOptions['plugins']> = [DeprecationsPlugin,
PromisesPlugin, TemplateParamsPlugin, AliasSortingPlugin]; this will convert the
inferred readonly tuple into the intended public plugin array type.
---
Duplicate comments:
In `@packages/unhead/src/legacy/index.ts`:
- Around line 21-35: The current remapping logic uses truthy checks and
therefore skips valid falsy values; change each conditional to check property
existence (e.g., Object.prototype.hasOwnProperty.call(tag.props, 'children') or
'children' in tag.props) before remapping so empty strings and boolean false are
handled and then perform the same assignment and delete (apply this for children
-> innerHTML, hid -> key, vmid -> key, and body -> tagPosition).
🪄 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: 13ad9409-f669-409b-b89d-d519e87ed6a2
📒 Files selected for processing (2)
packages/unhead/src/legacy/index.tspackages/vue/src/legacy.ts
- Add renderPriority → tagPriority mapping for v2 compat - Use 'body' in tag.props check so props.body=false still strips the prop
Verifies children/hid/vmid/body/renderPriority remap, body=false strips without moving tag, and hid dedupe across separate entries.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/unhead/src/legacy/index.ts (1)
21-31:⚠️ Potential issue | 🟡 MinorUse existence checks for the remaining legacy prop remaps.
Lines 21, 25, and 29 still use truthy checks, so valid falsy values like
children: '',hid: '', orvmid: ''are left ontag.propsinstead of being normalized. Match thebodybranch and switch these to key-existence checks before deleting the legacy prop.🔧 Proposed fix
- if (tag.props.children) { + if ('children' in tag.props) { tag.innerHTML = tag.props.children delete tag.props.children } - if (tag.props.hid) { + if ('hid' in tag.props) { tag.key = tag.props.hid delete tag.props.hid } - if (tag.props.vmid) { + if ('vmid' in tag.props) { tag.key = tag.props.vmid delete tag.props.vmid }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/unhead/src/legacy/index.ts` around lines 21 - 31, Change the truthy checks for legacy prop remaps to existence checks so falsy-but-valid values are normalized: for tag.props.children, tag.props.hid and tag.props.vmid use a key-existence test (e.g. Object.prototype.hasOwnProperty.call or the 'in' operator as used in the body branch) before assigning tag.innerHTML or tag.key and deleting the legacy prop; update the branches that handle children, hid and vmid to follow the same existence-check pattern as the body branch.
🤖 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/unhead/src/legacy/index.ts`:
- Around line 54-72: The module-level activeHead singleton is being overwritten
by createServerHead (assigning activeHead.value = _createServerHead(...)),
causing cross-request leakage; change createServerHead so it does NOT write to
the module-level activeHead and instead returns a fresh server head instance
(e.g., return _createServerHead<T>({...}) directly), and scope server heads to
the request (pass/store them in per-request context or use AsyncLocalStorage)
while keeping the module-level activeHead only for client-side
createHead/getActiveHead usage; update getActiveHead usage/docs to reflect it
only reads the global client head and ensure no server code relies on the module
singleton.
---
Duplicate comments:
In `@packages/unhead/src/legacy/index.ts`:
- Around line 21-31: Change the truthy checks for legacy prop remaps to
existence checks so falsy-but-valid values are normalized: for
tag.props.children, tag.props.hid and tag.props.vmid use a key-existence test
(e.g. Object.prototype.hasOwnProperty.call or the 'in' operator as used in the
body branch) before assigning tag.innerHTML or tag.key and deleting the legacy
prop; update the branches that handle children, hid and vmid to follow the same
existence-check pattern as the body branch.
🪄 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: 720fa27d-b110-43a0-97c8-4a8edb74dfdf
📒 Files selected for processing (1)
packages/unhead/src/legacy/index.ts
Comment on lines
+54
to
+72
| export const activeHead: { value: Unhead<any> | null } = { value: null } | ||
|
|
||
| export function getActiveHead<T extends Record<string, any> = ResolvableHead>(): Unhead<T> | null { | ||
| return activeHead.value | ||
| } | ||
|
|
||
| export function createHead<T extends Record<string, any> = ResolvableHead>(options: CreateClientHeadOptions = {}): Unhead<T> { | ||
| return activeHead.value = _createClientHead<T>({ | ||
| ...options, | ||
| plugins: [...legacyPlugins, ...(options.plugins || [])], | ||
| }) | ||
| } | ||
|
|
||
| export function createServerHead<T extends Record<string, any> = ResolvableHead>(options: Omit<CreateServerHeadOptions, 'propResolvers'> = {}): Unhead<T> { | ||
| return activeHead.value = _createServerHead<T>({ | ||
| ...options, | ||
| plugins: [...legacyPlugins, ...(options.plugins || [])], | ||
| }) | ||
| } |
There was a problem hiding this comment.
Don't store the server head in a module-level singleton.
activeHead is process-global, and Line 68 assigns every SSR instance into it. Under concurrent requests, getActiveHead() can return another request's head and bleed tags/meta across responses. Keep the global only for the client path, and scope the server active head per request or async context.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/unhead/src/legacy/index.ts` around lines 54 - 72, The module-level
activeHead singleton is being overwritten by createServerHead (assigning
activeHead.value = _createServerHead(...)), causing cross-request leakage;
change createServerHead so it does NOT write to the module-level activeHead and
instead returns a fresh server head instance (e.g., return
_createServerHead<T>({...}) directly), and scope server heads to the request
(pass/store them in per-request context or use AsyncLocalStorage) while keeping
the module-level activeHead only for client-side createHead/getActiveHead usage;
update getActiveHead usage/docs to reflect it only reads the global client head
and ensure no server code relies on the module singleton.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/unhead/src/legacy/index.ts (2)
67-69:⚠️ Potential issue | 🔴 CriticalDo not write SSR heads into module-global
activeHead.Line 68 stores each server-created head in process-global state, which can cause cross-request head leakage under concurrent SSR.
🛡️ Proposed fix
export function createServerHead<T extends Record<string, any> = ResolvableHead>(options: Omit<CreateServerHeadOptions, 'propResolvers'> = {}): Unhead<T> { - return activeHead.value = _createServerHead<T>({ + return _createServerHead<T>({ ...options, plugins: [...legacyPlugins, ...(options.plugins || [])], }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/unhead/src/legacy/index.ts` around lines 67 - 69, The function createServerHead currently assigns the newly created server head into the module-global activeHead (activeHead.value = _createServerHead...), which risks cross-request leakage; change it to simply return the result of _createServerHead<T>({...options}) without mutating activeHead.value so each call produces an isolated Unhead instance; keep references to createServerHead, activeHead and _createServerHead to locate the change and ensure no global state is written here (if per-request activation is needed, perform activeHead updates in request-scoped code instead).
21-31:⚠️ Potential issue | 🟡 MinorUse key-existence checks for all legacy prop remaps, not truthy checks.
Line 21, Line 25, and Line 29 still rely on truthiness, so falsy-but-present legacy values can bypass normalization and remain on
tag.props.🔧 Proposed fix
- if (tag.props.children) { + if ('children' in tag.props) { tag.innerHTML = tag.props.children delete tag.props.children } - if (tag.props.hid) { + if ('hid' in tag.props) { tag.key = tag.props.hid delete tag.props.hid } - if (tag.props.vmid) { + if ('vmid' in tag.props) { tag.key = tag.props.vmid delete tag.props.vmid }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/unhead/src/legacy/index.ts` around lines 21 - 31, The code currently uses truthy checks (if (tag.props.children), if (tag.props.hid), if (tag.props.vmid)) so falsy-but-present legacy props (e.g. empty string, 0, false) are ignored; change these to existence checks (e.g. Object.prototype.hasOwnProperty.call(tag.props, 'children') or 'children' in tag.props) and perform the same remap logic (assign tag.innerHTML / tag.key and delete the original prop) regardless of the value's truthiness; apply the same replacement for 'hid' and 'vmid' checks referencing tag.props.children, tag.innerHTML, tag.props.hid, tag.key, tag.props.vmid to ensure normalization always occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/unhead/src/legacy/index.ts`:
- Around line 67-69: The function createServerHead currently assigns the newly
created server head into the module-global activeHead (activeHead.value =
_createServerHead...), which risks cross-request leakage; change it to simply
return the result of _createServerHead<T>({...options}) without mutating
activeHead.value so each call produces an isolated Unhead instance; keep
references to createServerHead, activeHead and _createServerHead to locate the
change and ensure no global state is written here (if per-request activation is
needed, perform activeHead updates in request-scoped code instead).
- Around line 21-31: The code currently uses truthy checks (if
(tag.props.children), if (tag.props.hid), if (tag.props.vmid)) so
falsy-but-present legacy props (e.g. empty string, 0, false) are ignored; change
these to existence checks (e.g. Object.prototype.hasOwnProperty.call(tag.props,
'children') or 'children' in tag.props) and perform the same remap logic (assign
tag.innerHTML / tag.key and delete the original prop) regardless of the value's
truthiness; apply the same replacement for 'hid' and 'vmid' checks referencing
tag.props.children, tag.innerHTML, tag.props.hid, tag.key, tag.props.vmid to
ensure normalization always occurs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3d75478-a016-4105-a5ed-68028d32ece0
📒 Files selected for processing (3)
packages/unhead/src/legacy/index.tspackages/unhead/test/unit/plugins/deprecations.test.tspackages/vue/src/legacy.ts
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
N/A
❓ Type of change
📚 Description
The legacy entry points (
unhead/legacyand@unhead/vue/legacy) had regressed to emitting a deprecation warning only, leaving v2 consumers without thecreateHead/createServerHeadAPIs they rely on during migration. This restores the legacy factories (plusgetActiveHeadandcreateHeadCoreon the core entry) withDeprecationsPluginand the other v2 plugins pre-registered so v1/v2 tag props (children,hid,vmid,body) keep working. Also adds./legacyto theunheadpackageexportsandbuild.config.tsso the subpath is actually published.Summary by CodeRabbit
New Features
Tests