fix(bundler): preserve useSeoMeta import when sibling call is untransformable#749
Merged
Merged
Conversation
…formable `UseSeoMetaTransform` rewrote the static call to `useHead(...)` and stripped `useSeoMeta` from the import, even when another call site (dynamic first arg, spread properties, no args) kept using `useSeoMeta(...)`. The bundle then threw `ReferenceError: useSeoMeta is not defined` at runtime. Track untransformable call sites alongside transformable ones, and keep the original named import when either a value reference *or* an untransformable call requires it. Adds a runtime sandbox in the test that only binds names the rewritten import declares, so regressions surface as real `ReferenceError`s instead of passing silently under snapshot assertions.
📝 WalkthroughWalkthroughThe 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bundler/src/unplugin/UseSeoMetaTransform.ts (1)
270-295:⚠️ Potential issue | 🟡 MinorPre-existing limitation worth noting: aliased imports and multi-source imports.
The preservation logic at lines 282–283 keys both
valueReferencedanduntransformedCalleesby the original imported name. Two edge cases remain unaddressed:
- Aliased imports — for
import { useSeoMeta as SEOMETA } from 'unhead'with an untransformableSEOMETA(dynamic)call, the code re-addsuseSeoMeta(the original imported name) to the specifier list rather than preserving theuseSeoMeta as SEOMETAbinding. The call site would still throwReferenceError: SEOMETA is not defined. The existing "alt import as name" test covers only transformable cases.- Multiple imports of the same name from different sources — e.g.,
useSeoMetafrom bothunheadand@unhead/vuecollapse to a single name in the set, so an untransformable call from one source can cause over-preservation on the other's import.Neither is introduced by this PR — flagging only so it can be tracked / guarded with a follow-up test if desired.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bundler/src/unplugin/UseSeoMetaTransform.ts` around lines 270 - 295, The current preservation logic in the import rewrite loop (importRewrites, importNode, newSpecifiers, s.overwrite) uses valueReferenced and untransformedCallees keyed by the original imported name, which breaks aliased imports and multi-source imports; update the check to key by the local binding name (spec.local.name) and preserve alias syntax (emit "importedName as localName" when spec.local.name !== spec.imported.name) so that an untransformable call referencing an alias (e.g., SEOMETA) re-adds the correct aliased specifier instead of just the original name, and include the import source module (e.g., attach importNode.source.value) in the tracking keys or use a composite key [source, localName] to avoid collapsing identically named imports from different modules.
🧹 Nitpick comments (1)
packages/bundler/test/useSeoMetaTransform.test.ts (1)
17-39: Nice regression harness, but note the'unhead'-only source assumption.
runTransformedonly recognizesimport { ... } from 'unhead'via the regex on line 23 and is only consumed by the new test on line 113, which imports from'unhead'— so this is correct for the new tests. Worth being explicit that this helper is not usable for the@unhead/vue/#importstests if someone tries to repurpose it later. Also,importedcan contain aliased entries likeuseHead as x; those are silently dropped becausename in spieswould be false, which is the right behavior for current tests but another subtle assumption.Minor, non-blocking — either a short doc comment on the helper or tightening the filter would be enough.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bundler/test/useSeoMetaTransform.test.ts` around lines 17 - 39, The runTransformed helper currently only matches "import { ... } from 'unhead'" via the importMatch regex and drops aliased imports (e.g., "useHead as x") because imported entries are filtered by simple name presence in the spies map; update runTransformed to either (a) add a short doc comment above the function explaining the unhead-only assumption and that aliased imports are intentionally ignored, or (b) tighten parsing by normalizing imported names (split on "as" and trim the original specifier) so aliases map back to the spy keys before building params/args; reference runTransformed, importMatch, imported and spies when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/bundler/src/unplugin/UseSeoMetaTransform.ts`:
- Around line 270-295: The current preservation logic in the import rewrite loop
(importRewrites, importNode, newSpecifiers, s.overwrite) uses valueReferenced
and untransformedCallees keyed by the original imported name, which breaks
aliased imports and multi-source imports; update the check to key by the local
binding name (spec.local.name) and preserve alias syntax (emit "importedName as
localName" when spec.local.name !== spec.imported.name) so that an
untransformable call referencing an alias (e.g., SEOMETA) re-adds the correct
aliased specifier instead of just the original name, and include the import
source module (e.g., attach importNode.source.value) in the tracking keys or use
a composite key [source, localName] to avoid collapsing identically named
imports from different modules.
---
Nitpick comments:
In `@packages/bundler/test/useSeoMetaTransform.test.ts`:
- Around line 17-39: The runTransformed helper currently only matches "import {
... } from 'unhead'" via the importMatch regex and drops aliased imports (e.g.,
"useHead as x") because imported entries are filtered by simple name presence in
the spies map; update runTransformed to either (a) add a short doc comment above
the function explaining the unhead-only assumption and that aliased imports are
intentionally ignored, or (b) tighten parsing by normalizing imported names
(split on "as" and trim the original specifier) so aliases map back to the spy
keys before building params/args; reference runTransformed, importMatch,
imported and spies when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ac0048b-3fab-4007-af76-5aa6e8fffb12
📒 Files selected for processing (2)
packages/bundler/src/unplugin/UseSeoMetaTransform.tspackages/bundler/test/useSeoMetaTransform.test.ts
6 tasks
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.
❓ Type of change
📚 Description
UseSeoMetaTransformstrippeduseSeoMetafrom the import as soon as any call got rewritten touseHead, so an untransformable sibling call (dynamic first arg, spread properties) threwReferenceError: useSeoMeta is not definedat runtime. This PR tracks untransformable call sites and keeps the original named import when either a value reference or an untransformable call still needs it, with a sandboxed e2e test that only binds names the rewritten import declares so regressions surface as realReferenceErrors rather than slipping past snapshot assertions.