fix(compiler-sfc): import usage detection for non-inline templates #13858
Conversation
WalkthroughImplements template-language–aware import usage detection and template inlining in compileScript for inline SFC templates, including Pug. Refactors isImportUsed to accept template content/AST. Adjusts parse HMR reload check accordingly. Adds a test covering TS + script setup with Pug preprocessing to verify correct exposure of imports. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as SFC (script setup ts + template lang="pug")
participant CS as compileScript
participant CT as compileSFCTemplate
participant IU as isImportUsed
Dev->>CS: compile()
alt Inline template present
alt template.lang not set
CS->>IU: isImportUsed(local, template.content, template.ast)
else template.lang set (e.g., pug)
CS->>CT: compileSFCTemplate(template block, { inline: true, isTS, bindings })
CT-->>CS: { code, ast, map, ... }
CS->>IU: isImportUsed(local, template.content, compiled.ast)
end
IU-->>CS: usage boolean
CS->>CS: Register user imports accordingly
CS->>CT: compileSFCTemplate(...) for inlining
CT-->>CS: compiled template code/results
CS-->>Dev: output code
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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 |
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: |
![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 (3)
packages/compiler-sfc/src/script/importUsageCheck.ts (1)
28-32: Cache key may be too coarse (content-only)Results can theoretically vary with different parse options even when content is identical. Consider salting the cache key with a lightweight hash of relevant parse options (or a caller-provided cacheKey) to avoid cross-context collisions. Keep current behavior as default if adding an optional param.
packages/compiler-sfc/__tests__/compileScript/importUsageCheck.spec.ts (1)
268-295: Nice coverage for custom template languagesThe pug case validates components, interpolations, bindings, v-text, and ref usage. Consider adding an explicit negative check to ensure Thing1 is not exposed to make the intent self-verifying.
expect(content).toMatch( 'return { get Thing2() { return Thing2 }, get Thing3() { return Thing3 }, get Thing4() { return Thing4 }, get Thing5() { return Thing5 }, get Thing6() { return Thing6 } }', ) + expect(content).not.toMatch('get Thing1\\(\\)') // literal text shouldn't be exposedpackages/compiler-sfc/src/compileScript.ts (1)
250-250: Cache avoids repeated template compilation — minor naming nitOptional: rename to compiledTemplateCache for brevity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/compiler-sfc/__tests__/compileScript/__snapshots__/importUsageCheck.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
packages/compiler-sfc/__tests__/compileScript/importUsageCheck.spec.ts(1 hunks)packages/compiler-sfc/src/compileScript.ts(6 hunks)packages/compiler-sfc/src/parse.ts(1 hunks)packages/compiler-sfc/src/script/importUsageCheck.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/compiler-sfc/__tests__/compileScript/importUsageCheck.spec.ts (1)
packages/compiler-sfc/__tests__/utils.ts (1)
assertCode(27-42)
packages/compiler-sfc/src/parse.ts (1)
packages/compiler-sfc/src/script/importUsageCheck.ts (1)
isImportUsed(18-24)
packages/compiler-sfc/src/script/importUsageCheck.ts (2)
packages/compiler-core/src/ast.ts (2)
RootNode(104-121)SimpleExpressionNode(225-247)packages/compiler-sfc/src/cache.ts (1)
createCache(3-11)
packages/compiler-sfc/src/compileScript.ts (3)
packages/compiler-sfc/src/compileTemplate.ts (2)
SFCTemplateCompileResults(37-45)compileTemplate(107-160)packages/compiler-sfc/src/script/importUsageCheck.ts (1)
isImportUsed(18-24)packages/compiler-sfc/src/parse.ts (1)
SFCTemplateBlock(45-48)
⏰ 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). (4)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: test / e2e-test
🔇 Additional comments (6)
packages/compiler-sfc/src/script/importUsageCheck.ts (3)
18-24: API change aligns with new call sitesSwitching isImportUsed to accept (content, ast) is sound and makes the dependency explicit. No functional concerns here.
39-39: Traversal entrypoint looks goodWalking ast.children directly is correct now that a concrete RootNode is required.
98-103: Good fix: strip leading _ctx. for simple identifiersThis improves identifier matching for static simple expressions without over-matching.
packages/compiler-sfc/src/compileScript.ts (3)
12-12: Imports for SFCTemplate types — LGTM*Types are correctly imported for downstream usage.
Also applies to: 40-41
921-923: Unified inlining via compileSFCTemplate — LGTMConsistent path for inline and detection flows; source maps and helpers remain intact.
312-333: The scripts above will locate and inspect the relevant definitions and usages to verify whetherpreprocessLangis handled correctly in both inline and non-inline flows. Let me know when you have the results.
| !sfc.template.src | ||
| ) { | ||
| isUsedInTemplate = isImportUsed(local, sfc) | ||
| if (!sfc.template.lang) { | ||
| isUsedInTemplate = isImportUsed( | ||
| local, | ||
| sfc.template.content, | ||
| sfc.template.ast!, | ||
| ) | ||
| } else { | ||
| customTemplateLangCompiledSFC ||= compileSFCTemplate(sfc.template) | ||
| isUsedInTemplate = isImportUsed( | ||
| local, | ||
| sfc.template.content, | ||
| customTemplateLangCompiledSFC.ast!, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: using preprocessed AST with raw (un-preprocessed) content can mis-detect template imports
When lang is set (e.g., pug), compileSFCTemplate returns an AST for the preprocessed template, but content passed to isImportUsed is the raw SFC template (pug). This mismatch can lead to false negatives/positives. Also, ast may be undefined on preprocessing failure; defaulting to “used” is safer in dev/non-inline to avoid runtime breakage.
Apply the following:
- if (!sfc.template.lang) {
- isUsedInTemplate = isImportUsed(
- local,
- sfc.template.content,
- sfc.template.ast!,
- )
- } else {
- customTemplateLangCompiledSFC ||= compileSFCTemplate(sfc.template)
- isUsedInTemplate = isImportUsed(
- local,
- sfc.template.content,
- customTemplateLangCompiledSFC.ast!,
- )
- }
+ if (!sfc.template.lang) {
+ isUsedInTemplate = sfc.template.ast
+ ? isImportUsed(local, sfc.template.content, sfc.template.ast)
+ : true
+ } else {
+ const compiled =
+ (customTemplateLangCompiledSFC ||= compileSFCTemplate(sfc.template))
+ isUsedInTemplate = compiled.ast
+ ? isImportUsed(local, compiled.source, compiled.ast)
+ : true
+ }📝 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.
| !sfc.template.src | |
| ) { | |
| isUsedInTemplate = isImportUsed(local, sfc) | |
| if (!sfc.template.lang) { | |
| isUsedInTemplate = isImportUsed( | |
| local, | |
| sfc.template.content, | |
| sfc.template.ast!, | |
| ) | |
| } else { | |
| customTemplateLangCompiledSFC ||= compileSFCTemplate(sfc.template) | |
| isUsedInTemplate = isImportUsed( | |
| local, | |
| sfc.template.content, | |
| customTemplateLangCompiledSFC.ast!, | |
| ) | |
| } | |
| } | |
| !sfc.template.src | |
| ) { | |
| if (!sfc.template.lang) { | |
| isUsedInTemplate = sfc.template.ast | |
| ? isImportUsed(local, sfc.template.content, sfc.template.ast) | |
| : true | |
| } else { | |
| const compiled = | |
| (customTemplateLangCompiledSFC ||= compileSFCTemplate(sfc.template)) | |
| isUsedInTemplate = compiled.ast | |
| ? isImportUsed(local, compiled.source, compiled.ast) | |
| : true | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/compiler-sfc/src/compileScript.ts around lines 267 to 283, the code
calls isImportUsed with the raw SFC template content when a custom template lang
is used, but passes the preprocessed AST — this mismatch can mis-detect imports
and also assumes ast always exists; change the call to use the preprocessed
template content together with its AST when lang is set (i.e., use
customTemplateLangCompiledSFC.content and .ast), and if compileSFCTemplate fails
to produce an ast, avoid a false negative by defaulting isUsedInTemplate to true
(at least for dev/non-inline cases) so runtime behavior isn’t broken.
| if ( | ||
| !prevImports[key].isUsedInTemplate && | ||
| isImportUsed(key, next.template!.content!, next.template!.ast!) | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential runtime crash when template/AST is absent
next.template or next.template.ast can be null (e.g., external src templates). The non-null assertions will throw at runtime during HMR checks. Guard before calling isImportUsed.
Apply this diff:
- if (
- !prevImports[key].isUsedInTemplate &&
- isImportUsed(key, next.template!.content!, next.template!.ast!)
- ) {
+ // skip when there is no in-file template or no parsed AST (e.g. src=)
+ if (!next.template || !next.template.ast) {
+ continue
+ }
+ if (
+ !prevImports[key].isUsedInTemplate &&
+ isImportUsed(key, next.template.content, next.template.ast)
+ ) {
return true
}Optionally short-circuit earlier:
) {
return false
}
+ if (!next.template || !next.template.ast) {
+ return false
+ }📝 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.
| if ( | |
| !prevImports[key].isUsedInTemplate && | |
| isImportUsed(key, next.template!.content!, next.template!.ast!) | |
| ) { | |
| // skip when there is no in-file template or no parsed AST (e.g. src=) | |
| if (!next.template || !next.template.ast) { | |
| continue | |
| } | |
| if ( | |
| !prevImports[key].isUsedInTemplate && | |
| isImportUsed(key, next.template.content, next.template.ast) | |
| ) { | |
| return true | |
| } |
🤖 Prompt for AI Agents
In packages/compiler-sfc/src/parse.ts around lines 451 to 454, the code uses
non-null assertions on next.template and next.template.ast which can be null and
cause runtime crashes; guard these properties before calling isImportUsed by
first checking that next.template and next.template.ast exist (e.g., if
(next.template && next.template.ast) ) and only then call isImportUsed, or
short-circuit earlier so imports marked as unused when the template/AST is
absent; update the conditional to avoid using ! and .! on possibly null values.
close #12542
Improve detection of imported binding usage in template for non-inline SFC compilation mode.
Sample code
App.vue
decls.ts
In dev/non-inline mode before this change:
After change; builds/runs as expected:
no change in behavior in prod/inline mode, sample code works before and after change
Summary by CodeRabbit