From d66d6f26339cf866c0f9e1b14d7eb5ad0d5a0e76 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Mon, 29 Jan 2024 18:35:15 -0500 Subject: [PATCH 1/2] Looser import/export elision blocking in visitElidableStatement --- src/compiler/transformers/ts.ts | 69 +++++++++++++++++-- src/testRunner/unittests/customTransforms.ts | 43 ++++++++++++ ...importDeclarationBeforeTransformElision.js | 6 ++ 3 files changed, 112 insertions(+), 6 deletions(-) create mode 100644 tests/baselines/reference/customTransforms/importDeclarationBeforeTransformElision.js diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 0548a5bf0da9c..a0a997e3444d5 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -86,7 +86,10 @@ import { isComputedPropertyName, isDecorator, isElementAccessExpression, + isEntityName, isEnumConst, + isExportAssignment, + isExportDeclaration, isExportOrDefaultModifier, isExportSpecifier, isExpression, @@ -96,6 +99,8 @@ import { isHeritageClause, isIdentifier, isImportClause, + isImportDeclaration, + isImportEqualsDeclaration, isImportSpecifier, isInJSFile, isInstantiatedModule, @@ -425,13 +430,65 @@ export function transformTypeScript(context: TransformationContext) { } } - function visitElidableStatement(node: ImportDeclaration | ImportEqualsDeclaration | ExportAssignment | ExportDeclaration): VisitResult { + /** + * Determines whether import/export elision is blocked for this statement. + * + * @description + * We generally block import/export elision if the statement was modified by a `before` custom + * transform, although we will continue to allow it if the statement hasn't replaced a node of a different kind and + * as long as the local bindings for the declarations are unchanged. + */ + function isElisionBlocked(node: ImportDeclaration | ImportEqualsDeclaration | ExportAssignment | ExportDeclaration) { const parsed = getParseTreeNode(node); - if (parsed !== node) { - // If the node has been transformed by a `before` transformer, perform no ellision on it - // As the type information we would attempt to lookup to perform ellision is potentially unavailable for the synthesized nodes - // We do not reuse `visitorWorker`, as the ellidable statement syntax kinds are technically unrecognized by the switch-case in `visitTypeScript`, - // and will trigger debug failures when debug verbosity is turned up + if (parsed === node || isExportAssignment(node)) { + return false; + } + + if (!parsed || parsed.kind !== node.kind) { + // no longer safe to elide as the declaration was replaced with a node of a different kind + return true; + } + + switch (node.kind) { + case SyntaxKind.ImportDeclaration: + Debug.assertNode(parsed, isImportDeclaration); + if (node.importClause !== parsed.importClause) { + return true; // no longer safe to elide as the import clause has changed + } + if (node.attributes !== parsed.attributes) { + return true; // no longer safe to elide as the import attributes have changed + } + break; + case SyntaxKind.ImportEqualsDeclaration: + Debug.assertNode(parsed, isImportEqualsDeclaration); + if (node.name !== parsed.name) { + return true; // no longer safe to elide as local binding has changed + } + if (node.isTypeOnly !== parsed.isTypeOnly) { + return true; // no longer safe to elide as `type` modifier has changed + } + if (node.moduleReference !== parsed.moduleReference && (isEntityName(node.moduleReference) || isEntityName(parsed.moduleReference))) { + return true; // no longer safe to elide as EntityName reference has changed. + } + break; + case SyntaxKind.ExportDeclaration: + Debug.assertNode(parsed, isExportDeclaration); + if (node.exportClause !== parsed.exportClause) { + return true; // no longer safe to elide as the export clause has changed + } + if (node.attributes !== parsed.attributes) { + return true; // no longer safe to elide as the export attributes have changed + } + break; + } + + return false; + } + + function visitElidableStatement(node: ImportDeclaration | ImportEqualsDeclaration | ExportAssignment | ExportDeclaration): VisitResult { + if (isElisionBlocked(node)) { + // We do not reuse `visitorWorker`, as the ellidable statement syntax kinds are technically unrecognized by + // the switch-case in `visitTypeScript`, and will trigger debug failures when debug verbosity is turned up. if (node.transformFlags & TransformFlags.ContainsTypeScript) { // This node contains TypeScript, so we should visit its children. return visitEachChild(node, visitor, context); diff --git a/src/testRunner/unittests/customTransforms.ts b/src/testRunner/unittests/customTransforms.ts index c401275203d63..36ee965870884 100644 --- a/src/testRunner/unittests/customTransforms.ts +++ b/src/testRunner/unittests/customTransforms.ts @@ -164,4 +164,47 @@ describe("unittests:: customTransforms", () => { }, ], }, { sourceMap: true, outFile: "source.js" }); + + emitsCorrectly("importDeclarationBeforeTransformElision", [ + { + file: "a.ts", + text: "export type A = string;", + }, + { + file: "index.ts", + text: "import { A } from './a.js';\nexport { A } from './a.js';", + }, + ], { + before: [ + context => { + const { factory } = context; + return (s: ts.SourceFile) => ts.visitEachChild(s, visitor, context); + + function visitor(node: ts.Node): ts.Node { + if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) + return factory.updateImportDeclaration( + node, + node.modifiers, + node.importClause, + factory.createStringLiteral(node.moduleSpecifier.text), + node.attributes + ); + + if (ts.isExportDeclaration(node) && node.moduleSpecifier && ts.isStringLiteral(node.moduleSpecifier)) + return factory.updateExportDeclaration( + node, + node.modifiers, + node.isTypeOnly, + node.exportClause, + factory.createStringLiteral(node.moduleSpecifier.text), + node.attributes + ); + return node; + } + } + ] + }, { + target: ts.ScriptTarget.ESNext, + module: ts.ModuleKind.ESNext + }); }); diff --git a/tests/baselines/reference/customTransforms/importDeclarationBeforeTransformElision.js b/tests/baselines/reference/customTransforms/importDeclarationBeforeTransformElision.js new file mode 100644 index 0000000000000..e3ecd13825ab6 --- /dev/null +++ b/tests/baselines/reference/customTransforms/importDeclarationBeforeTransformElision.js @@ -0,0 +1,6 @@ +// [a.js] +export {}; + + +// [index.js] +export {}; From 418e911ee77300ee08885412ca7e4437e324dcae Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Tue, 30 Jan 2024 10:49:00 -0500 Subject: [PATCH 2/2] Formatting --- src/testRunner/unittests/customTransforms.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/testRunner/unittests/customTransforms.ts b/src/testRunner/unittests/customTransforms.ts index 36ee965870884..19d9d44562c7c 100644 --- a/src/testRunner/unittests/customTransforms.ts +++ b/src/testRunner/unittests/customTransforms.ts @@ -181,30 +181,32 @@ describe("unittests:: customTransforms", () => { return (s: ts.SourceFile) => ts.visitEachChild(s, visitor, context); function visitor(node: ts.Node): ts.Node { - if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) + if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { return factory.updateImportDeclaration( node, node.modifiers, node.importClause, factory.createStringLiteral(node.moduleSpecifier.text), - node.attributes + node.attributes, ); + } - if (ts.isExportDeclaration(node) && node.moduleSpecifier && ts.isStringLiteral(node.moduleSpecifier)) + if (ts.isExportDeclaration(node) && node.moduleSpecifier && ts.isStringLiteral(node.moduleSpecifier)) { return factory.updateExportDeclaration( node, node.modifiers, node.isTypeOnly, node.exportClause, factory.createStringLiteral(node.moduleSpecifier.text), - node.attributes + node.attributes, ); + } return node; } - } - ] + }, + ], }, { target: ts.ScriptTarget.ESNext, - module: ts.ModuleKind.ESNext + module: ts.ModuleKind.ESNext, }); });