-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
chore(compiler): Update Babel 8 #13937
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?
Conversation
Update Babel from v7 to v8 BREAKING CHANGE: Node types updated to Babel v8 definitions.
WalkthroughBabel-related upgrades to v8 beta prompted AST shape and parser plugin updates across compiler-sfc source and tests. Numerous references switched from typeParameters to typeArguments, enum members moved to body.members, and TS heritage/types handling was revised. Parsing defaults were simplified, a test was removed, a utility’s public signature changed, and workspace config updated. Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
Here's a test I'm not sure is expected, because its input is not valid TS code. packages\compiler-sfc_tests_\compileScript\resolveType.spec.ts test('template string type', () => {
expect(
resolve(`
type T = 'foo' | 'bar'
type S = 'x' | 'y'
defineProps<{
[\`_\${T}_\${S}_\`]: string
}>()
`).props,
).toStrictEqual({
_foo_x_: ['String'],
_foo_y_: ['String'],
_bar_x_: ['String'],
_bar_y_: ['String'],
})
}) type T = 'foo' | 'bar'
type S = 'x' | 'y'
defineProps<{
[`_${T}_${S}_`]: string
}>()
|
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/compiler-sfc/src/script/context.ts (1)
181-188
: Avoid forcing legacy decorators when modern decorators are configured
userPlugins.includes('decorators')
only catches bare string entries. If a caller passes the usual tuple form (['decorators', { decoratorsBeforeExport: true }]
), the check fails and we still push'decorators-legacy'
. Babel throws when both'decorators'
and'decorators-legacy'
are enabled ("You cannot use the legacy decorators plugin with the decorators plugin."
), so this breaks SFC parsing for any consumer configuring decorators with options. Please inspect tuple entries before adding the legacy plugin.- if (!userPlugins || !userPlugins.includes('decorators')) { - plugins.push('decorators-legacy') - } + const hasDecorators = + userPlugins?.some(p => (Array.isArray(p) ? p[0] : p) === 'decorators') ?? + false + if (!hasDecorators) { + plugins.push('decorators-legacy') + }packages/compiler-sfc/src/script/utils.ts (1)
697-755
: Guard against missing/insufficient type arguments in builtins.resolveBuiltin assumes node.typeArguments exists and has enough params; malformed input will throw. Add explicit checks and surface a helpful error via ctx.error().
Apply this diff:
function resolveBuiltin( ctx: TypeResolveContext, node: TSTypeReference | TSInterfaceHeritage, name: GetSetType<typeof SupportedBuiltinsSet>, scope: TypeScope, typeParameters?: Record<string, Node>, ): ResolvedElements { + if (!('typeArguments' in node) || !node.typeArguments || !node.typeArguments.params?.length) { + return ctx.error(`${name}<T> requires at least one type argument`, node as unknown as Node, scope) + } + if ((name === 'Pick' || name === 'Omit') && node.typeArguments.params.length < 2) { + return ctx.error(`${name}<T, K> requires two type arguments`, node as unknown as Node, scope) + } - const t = resolveTypeElements( + const t = resolveTypeElements( ctx, - node.typeArguments!.params[0], + node.typeArguments.params[0], scope, typeParameters, ) switch (name) { case 'Partial': { @@ case 'Pick': { - const picked = resolveStringType( + const picked = resolveStringType( ctx, - node.typeArguments!.params[1], + node.typeArguments.params[1], scope, typeParameters, ) @@ case 'Omit': - const omitted = resolveStringType( + const omitted = resolveStringType( ctx, - node.typeArguments!.params[1], + node.typeArguments.params[1], scope, typeParameters, )
🧹 Nitpick comments (1)
packages/compiler-sfc/src/script/utils.ts (1)
1533-1587
: Use the fromId parameter consistently.In mergeNamespaces the else-if checks from.id.type instead of fromId.type, unlike the toId branch. Minor readability fix.
Apply this diff:
- } else if (from.id.type === 'TSQualifiedName') { + } else if (fromId.type === 'TSQualifiedName') {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/compiler-sfc/__tests__/__snapshots__/compileScript.spec.ts.snap
is excluded by!**/*.snap
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
packages/compiler-core/__tests__/transforms/transformExpressions.spec.ts
(1 hunks)packages/compiler-sfc/__tests__/compileScript.spec.ts
(0 hunks)packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts
(1 hunks)packages/compiler-sfc/__tests__/utils.ts
(1 hunks)packages/compiler-sfc/src/compileScript.ts
(1 hunks)packages/compiler-sfc/src/script/context.ts
(2 hunks)packages/compiler-sfc/src/script/defineEmits.ts
(3 hunks)packages/compiler-sfc/src/script/defineModel.ts
(1 hunks)packages/compiler-sfc/src/script/defineOptions.ts
(1 hunks)packages/compiler-sfc/src/script/defineProps.ts
(1 hunks)packages/compiler-sfc/src/script/resolveType.ts
(33 hunks)packages/compiler-sfc/src/script/utils.ts
(2 hunks)pnpm-workspace.yaml
(1 hunks)scripts/inline-enums.js
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/compiler-sfc/tests/compileScript.spec.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/compiler-sfc/src/script/utils.ts (1)
packages/compiler-core/src/ast.ts (1)
Node
(70-73)
packages/compiler-sfc/src/script/resolveType.ts (2)
packages/compiler-sfc/src/script/utils.ts (1)
getId
(75-83)packages/compiler-core/src/ast.ts (1)
Node
(70-73)
🔇 Additional comments (15)
scripts/inline-enums.js (1)
93-94
: Enum traversal updated for Babel 8Switching to
decl.body.members
matches theTSEnumBody
reshape introduced in the Babel 8 betas, soscanEnums
continues to collect every member. Looks good.packages/compiler-sfc/src/script/utils.ts (9)
9-11
: Imports updated for TS qualified names — LGTM.Adding TSEntityName and TSQualifiedName is appropriate for Babel 8 AST.
654-685
: Nice addition: supports TSTemplateLiteralType.This mirrors TemplateLiteral handling and unblocks string key inference with TS template literal types.
842-858
: getReferenceName updates look right.Accounting for TSInterfaceHeritage and returning a path for TSQualifiedName matches the new AST.
871-887
: Helper is solid.pathToQualifiedName/qualifiedNameToPath pair is coherent and will help namespace handling.
2026-2040
: Enum member access updated — LGTM.Switching to node.body.members matches Babel 8’s enum shape.
2053-2058
: PropType/MethodSignature extraction updates look correct.Using returnType/typeArguments for reverse inference aligns with earlier changes.
Also applies to: 2096-2106
463-469
: Babel 8 adopts flattened TSMappedType fields
Babel 8’s TSMappedType exposeskey
andconstraint
directly (no morenode.typeParameter
), so existingnode.key
/node.constraint
usage is correct.
73-83
: Confirm getId’s TSQualifiedName behavior is intentional. All call sites either operate on Identifier or StringLiteral, or handle nested module names via moduleDeclToScope; none rely on the rightmost segment.
889-903
: No changes needed for TSModuleDeclaration global check.
Babel 8’s @babel/types definesTSModuleDeclaration
with a requiredkind
field ("global" | "module" | "namespace"
), so checkingstmt.kind === 'global'
correctly handlesdeclare global {}
. (babel.dev)packages/compiler-sfc/src/script/resolveType.ts (2)
209-227
: TSInterfaceHeritage support wired correctly.Handling extends via TSInterfaceHeritage and switching to node.typeArguments is consistent with Babel 8.
Also applies to: 271-281
291-307
: TSImportType.argument is always a StringLiteral per Babel AST, sonode.argument.literal
access is safe. (babeljs.io)packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts (1)
1656-1656
: Align CallExpression generics with Babel 8 ASTLine 1656 now reads the defineProps type argument from
typeArguments
, which matches Babel 8’s rename of call-expression generics. This keeps the test helper in sync with the new parser output. (babeljs.io)packages/compiler-sfc/src/script/defineEmits.ts (2)
32-41
: AdopttypeArguments
for defineEmits type payloadLines 32-41 switch the macro handling over to
node.typeArguments
, aligning with Babel 8’s AST schema for generic call expressions so we keep recognizing type-onlydefineEmits
declarations. (babeljs.io)
78-99
: Update emit signature traversal toparams
Lines 78-99 migrate from
parameters
toparams
, which is the new field name for TS call signatures and function types in Babel 8. This keeps runtime emit extraction working against the upgraded AST. (babel.dev)
plugins: ['typescript'], | ||
}) |
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.
🧩 Analysis chain
Babel 8 parser config looks correct.
Dropping importAttributes here aligns with the PR direction; TS-only parsing is fine for assertCode().
Run a quick repo scan to ensure no generated code or tests still rely on import attributes (assert/with):
🏁 Script executed:
#!/bin/bash
rg -nP -S 'importAttributes|\\bassert\\s*\\{|\\bwith\\s*\\{' --glob '!node_modules/**'
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Re-run search across all files (including those in .gitignore) to catch any usage of importAttributes, assert {, or with {,
# excluding node_modules and dist directories.
rg -nP --no-ignore-vcs --glob '!node_modules/**' --glob '!dist/**' 'importAttributes|\bassert\s*\{|\bwith\s*\{'
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Fallback search using grep across TS/JS/Vue files to catch any usage of importAttributes, assert {, or with {,
# excluding node_modules and dist directories.
find . -type f \( -name '*.ts' -o -name '*.js' -o -name '*.vue' \) \
-not -path './node_modules/*' -not -path './dist/*' \
-exec grep -nE 'importAttributes|assert\s*\{|with\s*\{' {} +
Length of output: 485
Restore importAttributes support or update tests
Tests in packages/compiler-sfc/tests/compileScript.spec.ts (lines 1501 & 1509) still use
import { foo } from './foo.js' with { type: 'foobar' }
import { foo } from './foo.js' assert { type: 'foobar' }
Dropping the importAttributes
parser plugin will break these. Either re-include it for compileSFCScript
or remove/adjust those tests.
🤖 Prompt for AI Agents
packages/compiler-sfc/__tests__/utils.ts around lines 32-33: the parser plugins
list removed support for import attributes which breaks tests that use "with {
type: '...'}" / "assert { type: '...'}"; restore the 'importAttributes' parser
plugin in the plugins array passed to compileSFCScript (or to whichever
parse/transform helper is used here) so import attribute syntax is accepted, or
if you prefer to change tests instead, update the tests in
packages/compiler-sfc/__tests__/compileScript.spec.ts to stop using import
attributes; prefer restoring 'importAttributes' in the plugins list to minimally
change behavior and keep existing tests working.
This PR updates Babel 7 to Babel 8.
This appears to be a breaking change, so it may not be merged at this time.
Ref: babel/babel#17530
cc @nicolo-ribaudo @JLHwung
Summary by CodeRabbit