-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
feat(compiler-core): consider component import names for resolving setup component references #13624
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
base: main
Are you sure you want to change the base?
feat(compiler-core): consider component import names for resolving setup component references #13624
Conversation
…tup component references
WalkthroughThe changes introduce a new mechanism for tracking "known components" in the Vue compiler's transformation and code generation stages. This includes adding a Changes
Sequence Diagram(s)sequenceDiagram
participant SFCCompiler as SFC Compiler
participant ScriptCompile as compileScript
participant Transform as createTransformContext
participant TemplateCompile as compileTemplate
SFCCompiler->>ScriptCompile: Compile <script setup>
ScriptCompile->>ScriptCompile: Identify user imports
ScriptCompile->>ScriptCompile: Add known components to knownComponents set
ScriptCompile->>TemplateCompile: Pass knownComponents in compilerOptions
TemplateCompile->>Transform: createTransformContext(knownComponents)
Transform->>Transform: Provide knownComponents to transform context
TemplateCompile->>TemplateCompile: Use resolveSetupReference with isComponent flag
TemplateCompile->>TemplateCompile: Prefer knownComponents for component tags
Assessment against linked issues
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
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: 0
🧹 Nitpick comments (1)
packages/compiler-core/src/transforms/transformElement.ts (1)
334-350
: Excellent component prioritization logic with minor optimization opportunity.The implementation correctly prioritizes known components when
isComponent
is true by checking each name variant against theknownComponents
set before falling back to the original binding resolution.Consider using optional chaining for cleaner code:
- isKnownComponent: - context.knownComponents && context.knownComponents.has(name), + isKnownComponent: context.knownComponents?.has(name),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/compiler-sfc/__tests__/__snapshots__/compileScript.spec.ts.snap
is excluded by!**/*.snap
📒 Files selected for processing (6)
packages/compiler-core/src/codegen.ts
(1 hunks)packages/compiler-core/src/options.ts
(1 hunks)packages/compiler-core/src/transform.ts
(2 hunks)packages/compiler-core/src/transforms/transformElement.ts
(3 hunks)packages/compiler-sfc/__tests__/compileScript.spec.ts
(1 hunks)packages/compiler-sfc/src/compileScript.ts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/compiler-sfc/__tests__/compileScript.spec.ts (1)
packages/compiler-sfc/__tests__/utils.ts (1)
assertCode
(27-42)
packages/compiler-core/src/codegen.ts (1)
packages/compiler-core/src/options.ts (1)
CodegenOptions
(311-353)
🪛 Biome (1.9.4)
packages/compiler-core/src/transforms/transformElement.ts
[error] 338-338: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ 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: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
🔇 Additional comments (10)
packages/compiler-core/src/options.ts (1)
202-207
: LGTM! Clean interface addition with clear documentation.The new
knownComponents
property is well-designed with appropriate typing and clear documentation explaining its purpose for component reference disambiguation.packages/compiler-core/src/codegen.ts (1)
123-126
: LGTM! Consistent exclusion of optional property.The exclusion of
knownComponents
from theRequired<CodegenOptions>
type follows the established pattern for other optional properties likebindingMetadata
andinline
.packages/compiler-sfc/__tests__/compileScript.spec.ts (1)
409-433
: LGTM! Comprehensive test case for component casing disambiguation.This test effectively validates the new feature by:
- Testing different casing styles (PascalCase, camelCase) for imported components
- Including conflicting local variables with similar names but different casing
- Verifying correct component resolution in the generated
_createVNode
calls- Following established testing patterns with
assertCode()
validationThe test provides good coverage for the component reference disambiguation functionality.
packages/compiler-core/src/transform.ts (2)
146-146
: LGTM! Clean parameter addition with backward compatibility.The addition of the
knownComponents
parameter with a default emptySet<string>()
maintains backward compatibility while enabling the new component disambiguation feature.
175-175
: LGTM! Proper propagation to transform context.The
knownComponents
property is correctly propagated to theTransformContext
object, following the established pattern for other options.packages/compiler-sfc/src/compileScript.ts (2)
729-744
: LGTM! Well-structured known components tracking.The logic correctly identifies known Vue components from user imports by checking for namespace imports, default imports from .vue files, and Vue package imports. The binding metadata assignment is also appropriate - known components get
SETUP_CONST
while potentially reactive imports getSETUP_MAYBE_REF
.
888-888
: Good integration with template compiler.Passing the
knownComponents
set to the template compiler options enables the enhanced component resolution in the template transformation phase.packages/compiler-core/src/transforms/transformElement.ts (3)
322-326
: Function signature enhancement enables component prioritization.The addition of the
isComponent
parameter allows the function to differentiate between component and directive resolution contexts, enabling more precise binding resolution.
289-289
: Appropriate component resolution context.Correctly passing
true
forisComponent
in component resolution scenarios enables the new prioritization logic for known components.Also applies to: 295-295
899-899
: Correct directive resolution context.Passing
false
forisComponent
in directive resolution maintains the original behavior since directives shouldn't prioritize component bindings.
close #9303
If the issue above is not considered a bug then this can be considered a breaking change, though a worthwhile one in my opinion.
Verified that the SFC playground example from the issue works as expected:

Summary by CodeRabbit
New Features
Bug Fixes
Tests
<script setup>
.