From c033dae04915d5dd83c0e174c6b0b6bc7bdf85ec Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 17 Dec 2024 20:20:57 -0500 Subject: [PATCH] fix: bump esrap, prevent malformed AST (#14742) * bump esrap, start fixing stuff * ignore type exports when looking for undefineds * sanitize stuff * changeset * fix * oops * try this? * prettier --- .changeset/purple-donuts-talk.md | 5 +++++ packages/svelte/package.json | 2 +- .../phases/1-parse/remove_typescript_nodes.js | 21 +++++++++---------- .../src/compiler/phases/2-analyze/index.js | 10 ++++++++- .../src/compiler/phases/3-transform/index.js | 4 ++++ packages/svelte/src/compiler/phases/scope.js | 6 ++++++ .../samples/typescript/main.svelte | 2 +- packages/svelte/tsconfig.json | 6 +++++- pnpm-lock.yaml | 12 +++++------ 9 files changed, 47 insertions(+), 21 deletions(-) create mode 100644 .changeset/purple-donuts-talk.md diff --git a/.changeset/purple-donuts-talk.md b/.changeset/purple-donuts-talk.md new file mode 100644 index 0000000000..2bb348cd19 --- /dev/null +++ b/.changeset/purple-donuts-talk.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: bump esrap, prevent malformed AST diff --git a/packages/svelte/package.json b/packages/svelte/package.json index a6eb2f4a8a..6603fe9b42 100644 --- a/packages/svelte/package.json +++ b/packages/svelte/package.json @@ -147,7 +147,7 @@ "aria-query": "^5.3.1", "axobject-query": "^4.1.0", "esm-env": "^1.2.1", - "esrap": "^1.2.3", + "esrap": "^1.3.1", "is-reference": "^3.0.3", "locate-character": "^3.0.0", "magic-string": "^0.30.11", diff --git a/packages/svelte/src/compiler/phases/1-parse/remove_typescript_nodes.js b/packages/svelte/src/compiler/phases/1-parse/remove_typescript_nodes.js index b968e95213..5e4fa231de 100644 --- a/packages/svelte/src/compiler/phases/1-parse/remove_typescript_nodes.js +++ b/packages/svelte/src/compiler/phases/1-parse/remove_typescript_nodes.js @@ -17,6 +17,16 @@ function remove_this_param(node, context) { /** @type {Visitors} */ const visitors = { + _(node, context) { + context.next(); + + // TODO there may come a time when we decide to preserve type annotations. + // until that day comes, we just delete them so they don't confuse esrap + delete node.typeAnnotation; + delete node.typeParameters; + delete node.returnType; + delete node.accessibility; + }, Decorator(node) { e.typescript_invalid_feature(node, 'decorators (related TSC proposal is not stage 4 yet)'); }, @@ -78,23 +88,12 @@ const visitors = { TSNonNullExpression(node, context) { return context.visit(node.expression); }, - TSTypeAnnotation() { - // This isn't correct, strictly speaking, and could result in invalid ASTs (like an empty statement within function parameters), - // but esrap, our printing tool, just ignores these AST nodes at invalid positions, so it's fine - return b.empty; - }, TSInterfaceDeclaration() { return b.empty; }, TSTypeAliasDeclaration() { return b.empty; }, - TSTypeParameterDeclaration() { - return b.empty; - }, - TSTypeParameterInstantiation() { - return b.empty; - }, TSEnumDeclaration(node) { e.typescript_invalid_feature(node, 'enums'); }, diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 042e88fa2f..d17cc34a5f 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -698,8 +698,16 @@ export function analyze_component(root, source, options) { } for (const node of analysis.module.ast.body) { - if (node.type === 'ExportNamedDeclaration' && node.specifiers !== null && node.source == null) { + if ( + node.type === 'ExportNamedDeclaration' && + // @ts-expect-error + node.exportKind !== 'type' && + node.specifiers !== null && + node.source == null + ) { for (const specifier of node.specifiers) { + // @ts-expect-error + if (specifier.exportKind === 'type') continue; if (specifier.local.type !== 'Identifier') continue; const binding = analysis.module.scope.get(specifier.local.name); diff --git a/packages/svelte/src/compiler/phases/3-transform/index.js b/packages/svelte/src/compiler/phases/3-transform/index.js index 8f6597ee12..d0739d6cc4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/index.js +++ b/packages/svelte/src/compiler/phases/3-transform/index.js @@ -33,12 +33,15 @@ export function transform_component(analysis, source, options) { : client_component(analysis, options); const js_source_name = get_source_name(options.filename, options.outputFilename, 'input.svelte'); + + // @ts-expect-error const js = print(program, { // include source content; makes it easier/more robust looking up the source map code // (else esrap does return null for source and sourceMapContent which may trip up tooling) sourceMapContent: source, sourceMapSource: js_source_name }); + merge_with_preprocessor_map(js, options, js_source_name); const css = @@ -92,6 +95,7 @@ export function transform_module(analysis, source, options) { } return { + // @ts-expect-error js: print(program, { // include source content; makes it easier/more robust looking up the source map code // (else esrap does return null for source and sourceMapContent which may trip up tooling) diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 3536dd6a18..20dcfa2821 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -433,7 +433,13 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { }, ImportDeclaration(node, { state }) { + // @ts-expect-error + if (node.importKind === 'type') return; + for (const specifier of node.specifiers) { + // @ts-expect-error + if (specifier.importKind === 'type') continue; + state.scope.declare(specifier.local, 'normal', 'import', node); } }, diff --git a/packages/svelte/tests/runtime-runes/samples/typescript/main.svelte b/packages/svelte/tests/runtime-runes/samples/typescript/main.svelte index 3b98dafa05..cc31350601 100644 --- a/packages/svelte/tests/runtime-runes/samples/typescript/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/typescript/main.svelte @@ -24,7 +24,7 @@ declare module 'foobar' {} namespace SomeNamespace { - export type Foo = true + export type Foo = true; } export function overload(a: boolean): boolean; diff --git a/packages/svelte/tsconfig.json b/packages/svelte/tsconfig.json index 380307901e..c9f0fb3b2b 100644 --- a/packages/svelte/tsconfig.json +++ b/packages/svelte/tsconfig.json @@ -40,5 +40,9 @@ "./tests/runtime-browser/test-ssr.ts", "./tests/*/samples/*/_config.js" ], - "exclude": ["./scripts/process-messages/templates/", "./src/compiler/optimizer/"] + "exclude": [ + "./scripts/process-messages/templates/", + "./scripts/_bundle.js", + "./src/compiler/optimizer/" + ] } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 250a03744e..1de6883a48 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -84,8 +84,8 @@ importers: specifier: ^1.2.1 version: 1.2.1 esrap: - specifier: ^1.2.3 - version: 1.2.3 + specifier: ^1.3.1 + version: 1.3.1 is-reference: specifier: ^3.0.3 version: 3.0.3 @@ -1111,8 +1111,8 @@ packages: resolution: {integrity: sha512-YQLXUplAwJgCydQ78IMJywZCceoqk1oH01OERdSAJc/7U2AylwjhSCLDEtqwg811idIS/9fIU5GjG73IgjKMVg==} engines: {node: '>=0.10'} - esrap@1.2.3: - resolution: {integrity: sha512-ZlQmCCK+n7SGoqo7DnfKaP1sJZa49P01/dXzmjCASSo04p72w8EksT2NMK8CEX8DhKsfJXANioIw8VyHNsBfvQ==} + esrap@1.3.1: + resolution: {integrity: sha512-KpAH3+QsDmtOP1KOW04CbD1PgzWsIHjB8tOCk3PCb8xzNGn8XkjI8zl80i09fmXdzQyaS8tcsKCCDzHF7AcowA==} esrecurse@4.3.0: resolution: {integrity: sha512-KmfKL3b6G+RXvP8N1vr3Tq1kL/oCFgn2NYXEtqP8/L3pKapUA4G8cFVaoF3SU323CD4XypR/ffioHmkti6/Tag==} @@ -3315,10 +3315,10 @@ snapshots: dependencies: estraverse: 5.3.0 - esrap@1.2.3: + esrap@1.3.1: dependencies: '@jridgewell/sourcemap-codec': 1.5.0 - '@types/estree': 1.0.6 + '@typescript-eslint/types': 8.2.0 esrecurse@4.3.0: dependencies: