From e67218531a8df7d6279b3e37725f93ec09c00a12 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Mon, 6 Apr 2020 16:03:12 -0700 Subject: [PATCH 1/3] Retain imports in declaration emit if they augment an export of the importing file --- src/compiler/checker.ts | 22 +++++++- src/compiler/emitter.ts | 1 + src/compiler/transformers/declarations.ts | 10 ++++ src/compiler/types.ts | 1 + ...mportingModuleAugmentationRetainsImport.js | 56 +++++++++++++++++++ ...ingModuleAugmentationRetainsImport.symbols | 46 +++++++++++++++ ...rtingModuleAugmentationRetainsImport.types | 46 +++++++++++++++ ...mportingModuleAugmentationRetainsImport.ts | 20 +++++++ 8 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.js create mode 100644 tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.symbols create mode 100644 tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.types create mode 100644 tests/cases/compiler/declarationEmitForModuleImportingModuleAugmentationRetainsImport.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index ffec8d3cb168b..d3842fb5fafae 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -36046,9 +36046,29 @@ namespace ts { return !node.locals ? [] : nodeBuilder.symbolTableToDeclarationStatements(node.locals, node, flags, tracker, bundled); } return !sym.exports ? [] : nodeBuilder.symbolTableToDeclarationStatements(sym.exports, node, flags, tracker, bundled); - } + }, + isImportRequierdByAugmentation, }; + function isImportRequierdByAugmentation(node: ImportDeclaration) { + const file = getSourceFileOfNode(node); + if (!file.symbol) return false; + const importTarget = getExternalModuleFileFromDeclaration(node); + if (!importTarget) return false; + if (importTarget === file) return false; + const exports = getExportsOfModule(file.symbol); + for (const s of arrayFrom(exports.values())) { + const merged = getMergedSymbol(s); + for (const d of merged.declarations) { + const declFile = getSourceFileOfNode(d); + if (declFile === importTarget) { + return true; + } + } + } + return false; + } + function isInHeritageClause(node: PropertyAccessEntityNameExpression) { return node.parent && node.parent.kind === SyntaxKind.ExpressionWithTypeArguments && node.parent.parent && node.parent.parent.kind === SyntaxKind.HeritageClause; } diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 3f52f4e9ee9e5..94cc11df05061 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -664,6 +664,7 @@ namespace ts { getSymbolOfExternalModuleSpecifier: notImplemented, isBindingCapturedByNode: notImplemented, getDeclarationStatementsForSourceFile: notImplemented, + isImportRequierdByAugmentation: notImplemented, }; /*@internal*/ diff --git a/src/compiler/transformers/declarations.ts b/src/compiler/transformers/declarations.ts index 743aa1b476932..ba2a9bdc18a35 100644 --- a/src/compiler/transformers/declarations.ts +++ b/src/compiler/transformers/declarations.ts @@ -717,6 +717,16 @@ namespace ts { rewriteModuleSpecifier(decl, decl.moduleSpecifier) ); } + // Augmentation of export depends on import + if (resolver.isImportRequierdByAugmentation(decl)) { + return updateImportDeclaration( + decl, + /*decorators*/ undefined, + decl.modifiers, + /*importClause*/ undefined, + rewriteModuleSpecifier(decl, decl.moduleSpecifier) + ); + } // Nothing visible } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index b0a0658130b7b..9f4f8b584f361 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -4004,6 +4004,7 @@ namespace ts { getSymbolOfExternalModuleSpecifier(node: StringLiteralLike): Symbol | undefined; isBindingCapturedByNode(node: Node, decl: VariableDeclaration | BindingElement): boolean; getDeclarationStatementsForSourceFile(node: SourceFile, flags: NodeBuilderFlags, tracker: SymbolTracker, bundled?: boolean): Statement[] | undefined; + isImportRequierdByAugmentation(decl: ImportDeclaration): boolean; } export const enum SymbolFlags { diff --git a/tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.js b/tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.js new file mode 100644 index 0000000000000..576b8dffbdef0 --- /dev/null +++ b/tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.js @@ -0,0 +1,56 @@ +//// [tests/cases/compiler/declarationEmitForModuleImportingModuleAugmentationRetainsImport.ts] //// + +//// [child1.ts] +import { ParentThing } from './parent'; + +declare module './parent' { + interface ParentThing { + add: (a: number, b: number) => number; + } +} + +export function child1(prototype: ParentThing) { + prototype.add = (a: number, b: number) => a + b; +} + +//// [parent.ts] +import { child1 } from './child1'; // this import should still exist in some form in the output, since it augments this module + +export class ParentThing implements ParentThing {} + +child1(ParentThing.prototype); + +//// [parent.js] +"use strict"; +exports.__esModule = true; +exports.ParentThing = void 0; +var child1_1 = require("./child1"); // this import should still exist in some form in the output, since it augments this module +var ParentThing = /** @class */ (function () { + function ParentThing() { + } + return ParentThing; +}()); +exports.ParentThing = ParentThing; +child1_1.child1(ParentThing.prototype); +//// [child1.js] +"use strict"; +exports.__esModule = true; +exports.child1 = void 0; +function child1(prototype) { + prototype.add = function (a, b) { return a + b; }; +} +exports.child1 = child1; + + +//// [parent.d.ts] +import './child1'; +export declare class ParentThing implements ParentThing { +} +//// [child1.d.ts] +import { ParentThing } from './parent'; +declare module './parent' { + interface ParentThing { + add: (a: number, b: number) => number; + } +} +export declare function child1(prototype: ParentThing): void; diff --git a/tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.symbols b/tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.symbols new file mode 100644 index 0000000000000..f7026a83b1b26 --- /dev/null +++ b/tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.symbols @@ -0,0 +1,46 @@ +=== tests/cases/compiler/child1.ts === +import { ParentThing } from './parent'; +>ParentThing : Symbol(ParentThing, Decl(child1.ts, 0, 8)) + +declare module './parent' { +>'./parent' : Symbol("tests/cases/compiler/parent", Decl(parent.ts, 0, 0), Decl(child1.ts, 0, 39)) + + interface ParentThing { +>ParentThing : Symbol(ParentThing, Decl(parent.ts, 0, 34), Decl(child1.ts, 2, 27)) + + add: (a: number, b: number) => number; +>add : Symbol(ParentThing.add, Decl(child1.ts, 3, 27)) +>a : Symbol(a, Decl(child1.ts, 4, 14)) +>b : Symbol(b, Decl(child1.ts, 4, 24)) + } +} + +export function child1(prototype: ParentThing) { +>child1 : Symbol(child1, Decl(child1.ts, 6, 1)) +>prototype : Symbol(prototype, Decl(child1.ts, 8, 23)) +>ParentThing : Symbol(ParentThing, Decl(child1.ts, 0, 8)) + + prototype.add = (a: number, b: number) => a + b; +>prototype.add : Symbol(ParentThing.add, Decl(child1.ts, 3, 27)) +>prototype : Symbol(prototype, Decl(child1.ts, 8, 23)) +>add : Symbol(ParentThing.add, Decl(child1.ts, 3, 27)) +>a : Symbol(a, Decl(child1.ts, 9, 21)) +>b : Symbol(b, Decl(child1.ts, 9, 31)) +>a : Symbol(a, Decl(child1.ts, 9, 21)) +>b : Symbol(b, Decl(child1.ts, 9, 31)) +} + +=== tests/cases/compiler/parent.ts === +import { child1 } from './child1'; // this import should still exist in some form in the output, since it augments this module +>child1 : Symbol(child1, Decl(parent.ts, 0, 8)) + +export class ParentThing implements ParentThing {} +>ParentThing : Symbol(ParentThing, Decl(parent.ts, 0, 34), Decl(child1.ts, 2, 27)) +>ParentThing : Symbol(ParentThing, Decl(parent.ts, 0, 34), Decl(child1.ts, 2, 27)) + +child1(ParentThing.prototype); +>child1 : Symbol(child1, Decl(parent.ts, 0, 8)) +>ParentThing.prototype : Symbol(ParentThing.prototype) +>ParentThing : Symbol(ParentThing, Decl(parent.ts, 0, 34), Decl(child1.ts, 2, 27)) +>prototype : Symbol(ParentThing.prototype) + diff --git a/tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.types b/tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.types new file mode 100644 index 0000000000000..bcb3f3a0666e9 --- /dev/null +++ b/tests/baselines/reference/declarationEmitForModuleImportingModuleAugmentationRetainsImport.types @@ -0,0 +1,46 @@ +=== tests/cases/compiler/child1.ts === +import { ParentThing } from './parent'; +>ParentThing : typeof ParentThing + +declare module './parent' { +>'./parent' : typeof import("tests/cases/compiler/parent") + + interface ParentThing { + add: (a: number, b: number) => number; +>add : (a: number, b: number) => number +>a : number +>b : number + } +} + +export function child1(prototype: ParentThing) { +>child1 : (prototype: ParentThing) => void +>prototype : ParentThing + + prototype.add = (a: number, b: number) => a + b; +>prototype.add = (a: number, b: number) => a + b : (a: number, b: number) => number +>prototype.add : (a: number, b: number) => number +>prototype : ParentThing +>add : (a: number, b: number) => number +>(a: number, b: number) => a + b : (a: number, b: number) => number +>a : number +>b : number +>a + b : number +>a : number +>b : number +} + +=== tests/cases/compiler/parent.ts === +import { child1 } from './child1'; // this import should still exist in some form in the output, since it augments this module +>child1 : (prototype: ParentThing) => void + +export class ParentThing implements ParentThing {} +>ParentThing : ParentThing + +child1(ParentThing.prototype); +>child1(ParentThing.prototype) : void +>child1 : (prototype: ParentThing) => void +>ParentThing.prototype : ParentThing +>ParentThing : typeof ParentThing +>prototype : ParentThing + diff --git a/tests/cases/compiler/declarationEmitForModuleImportingModuleAugmentationRetainsImport.ts b/tests/cases/compiler/declarationEmitForModuleImportingModuleAugmentationRetainsImport.ts new file mode 100644 index 0000000000000..5d6268864fe52 --- /dev/null +++ b/tests/cases/compiler/declarationEmitForModuleImportingModuleAugmentationRetainsImport.ts @@ -0,0 +1,20 @@ +// @declaration: true +// @filename: child1.ts +import { ParentThing } from './parent'; + +declare module './parent' { + interface ParentThing { + add: (a: number, b: number) => number; + } +} + +export function child1(prototype: ParentThing) { + prototype.add = (a: number, b: number) => a + b; +} + +// @filename: parent.ts +import { child1 } from './child1'; // this import should still exist in some form in the output, since it augments this module + +export class ParentThing implements ParentThing {} + +child1(ParentThing.prototype); \ No newline at end of file From 4c41e303597f5a1199792c774bd5bfe6bb9e6795 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Thu, 9 Apr 2020 16:08:12 -0700 Subject: [PATCH 2/3] (sp) --- src/compiler/checker.ts | 4 ++-- src/compiler/emitter.ts | 2 +- src/compiler/transformers/declarations.ts | 2 +- src/compiler/types.ts | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d3842fb5fafae..78a928e3c2f22 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -36047,10 +36047,10 @@ namespace ts { } return !sym.exports ? [] : nodeBuilder.symbolTableToDeclarationStatements(sym.exports, node, flags, tracker, bundled); }, - isImportRequierdByAugmentation, + isImportRequiredByAugmentation, }; - function isImportRequierdByAugmentation(node: ImportDeclaration) { + function isImportRequiredByAugmentation(node: ImportDeclaration) { const file = getSourceFileOfNode(node); if (!file.symbol) return false; const importTarget = getExternalModuleFileFromDeclaration(node); diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 94cc11df05061..d558ce785a867 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -664,7 +664,7 @@ namespace ts { getSymbolOfExternalModuleSpecifier: notImplemented, isBindingCapturedByNode: notImplemented, getDeclarationStatementsForSourceFile: notImplemented, - isImportRequierdByAugmentation: notImplemented, + isImportRequiredByAugmentation: notImplemented, }; /*@internal*/ diff --git a/src/compiler/transformers/declarations.ts b/src/compiler/transformers/declarations.ts index ba2a9bdc18a35..70f70a98c3e70 100644 --- a/src/compiler/transformers/declarations.ts +++ b/src/compiler/transformers/declarations.ts @@ -718,7 +718,7 @@ namespace ts { ); } // Augmentation of export depends on import - if (resolver.isImportRequierdByAugmentation(decl)) { + if (resolver.isImportRequiredByAugmentation(decl)) { return updateImportDeclaration( decl, /*decorators*/ undefined, diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 9f4f8b584f361..5e1b0a9fca86e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -4004,7 +4004,7 @@ namespace ts { getSymbolOfExternalModuleSpecifier(node: StringLiteralLike): Symbol | undefined; isBindingCapturedByNode(node: Node, decl: VariableDeclaration | BindingElement): boolean; getDeclarationStatementsForSourceFile(node: SourceFile, flags: NodeBuilderFlags, tracker: SymbolTracker, bundled?: boolean): Statement[] | undefined; - isImportRequierdByAugmentation(decl: ImportDeclaration): boolean; + isImportRequiredByAugmentation(decl: ImportDeclaration): boolean; } export const enum SymbolFlags { From c49a5986a78655ccb8fe44f3205ad9dba2665e36 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Thu, 9 Apr 2020 16:09:25 -0700 Subject: [PATCH 3/3] Check that a merge occurs, just because --- src/compiler/checker.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 78a928e3c2f22..d4a764be8a45c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -36058,11 +36058,13 @@ namespace ts { if (importTarget === file) return false; const exports = getExportsOfModule(file.symbol); for (const s of arrayFrom(exports.values())) { - const merged = getMergedSymbol(s); - for (const d of merged.declarations) { - const declFile = getSourceFileOfNode(d); - if (declFile === importTarget) { - return true; + if (s.mergeId) { + const merged = getMergedSymbol(s); + for (const d of merged.declarations) { + const declFile = getSourceFileOfNode(d); + if (declFile === importTarget) { + return true; + } } } }