-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
fix(compiler-sfc): check lang before attempt to compile script #13508
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
Conversation
WalkthroughRefactors script language detection to use new Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant compileScript
participant utils as isJS/isTS
participant Response
Caller->>compileScript: compile SFC (has <script lang="coffee">)
compileScript->>utils: isJS(lang), isTS(lang)
utils-->>compileScript: false / false
compileScript-->>Response: return original script block (lang="coffee", no scriptAst)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
@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: |
Size ReportBundles
Usages
|
❌ Deploy Preview for vue-sfc-playground failed. Why did it fail? →
|
/ecosystem-ci run |
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: 1
🧹 Nitpick comments (1)
packages/compiler-sfc/src/compileScript.ts (1)
216-219
: Early-return for non-JS/TS <script setup> is correct; consider edge case with both blocks non-JS/TS.If both <script> and <script setup> exist with the same custom lang, we currently return only scriptSetup and effectively drop the normal script. Is that intended? If not, we may need to surface both blocks to downstream tooling or enforce a single-block constraint for non-JS/TS.
Would you like me to draft a test covering “both blocks with lang='coffee'” and clarify expected behavior?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileScript.spec.ts
(1 hunks)packages/compiler-sfc/src/compileScript.ts
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/compiler-sfc/tests/compileScript.spec.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/compiler-sfc/src/compileScript.ts (3)
packages/compiler-sfc/src/script/utils.ts (2)
isJS
(125-126)isTS
(127-128)packages/compiler-sfc/src/script/context.ts (1)
ScriptCompileContext
(14-151)packages/compiler-sfc/src/script/normalScript.ts (1)
processNormalScript
(10-53)
🪛 Biome (2.1.2)
packages/compiler-sfc/src/compileScript.ts
[error] 221-221: Shouldn't redeclare 'ctx'. Consider to delete it or rename it.
'ctx' is defined here:
(lint/suspicious/noRedeclare)
🪛 GitHub Check: test / lint-and-test-dts
packages/compiler-sfc/src/compileScript.ts
[failure] 221-221:
Cannot redeclare block-scoped variable 'ctx'.
⏰ 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: test / unit-test
- GitHub Check: test / e2e-test
- GitHub Check: test / unit-test-windows
🔇 Additional comments (4)
packages/compiler-sfc/src/compileScript.ts (4)
59-65
: Good import additions for centralized lang detection.Using isJS/isTS from script/utils.ts is the right direction to keep language checks consistent across the codebase.
182-184
: LGTM: single boolean for JS/TS detection.isJSOrTS correctly captures both script and scriptSetup lang variants.
199-204
: Correct early-return for non-JS/TS normal <script>.This avoids unnecessary parsing and lets downstream tools handle custom langs.
209-214
: Duplicate lang-mismatch check; remove to avoid redundancy.This block repeats the same error thrown at Lines 185–190 and is unnecessary.
Apply this diff to drop the duplicate:
- if (script && scriptLang !== scriptSetupLang) { - throw new Error( - `[@vue/compiler-sfc] <script> and <script setup> must have the same ` + - `language type.`, - ) - }Likely an incorrect or invalid review comment.
📝 Ran ecosystem CI: Open
|
close #8368
Move the logic to avoid compilation if the script is not JS or TS before the file is actually attempted to be compiled.
I tested with a similar example to the reproduction link given in the issue:
App.vue:
vite.config.js:
Builds and functions as expected:

Summary by CodeRabbit