From dcd7de1a0cc72992356da233c363fe9512bae173 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 7 Oct 2020 16:26:18 -0700 Subject: [PATCH] Fix relative paths in commonjs decl emit w/property access ```js const x = require('./foo').y ``` was incorrectly using the unmangled require path as the temp name in emit: ``` import ./foo_1 = require('./foo') import x = ./foo_1.y ``` It now uses the imported identifier: ``` import x_1 = require('./foo') import x = x_1.y ``` Discovered while fixing #37832 --- src/compiler/checker.ts | 6 ++-- .../jsDeclarationsCommonjsRelativePath.js | 22 ++++++++++++ ...jsDeclarationsCommonjsRelativePath.symbols | 26 ++++++++++++++ .../jsDeclarationsCommonjsRelativePath.types | 35 +++++++++++++++++++ .../reference/jsDeclarationsTypeReferences.js | 4 +-- .../jsDeclarationsTypeReferences3.js | 4 +-- .../jsDeclarationsCommonjsRelativePath.ts | 13 +++++++ 7 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 tests/baselines/reference/jsDeclarationsCommonjsRelativePath.js create mode 100644 tests/baselines/reference/jsDeclarationsCommonjsRelativePath.symbols create mode 100644 tests/baselines/reference/jsDeclarationsCommonjsRelativePath.types create mode 100644 tests/cases/conformance/jsdoc/declarations/jsDeclarationsCommonjsRelativePath.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 159ca30a18898..a38afbee6ff19 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6788,16 +6788,16 @@ namespace ts { if (isPropertyAccessExpression((node as VariableDeclaration).initializer!)) { // const x = require('y').z const initializer = (node as VariableDeclaration).initializer! as PropertyAccessExpression; // require('y').z - const uniqueName = factory.createUniqueName((getExternalModuleRequireArgument(node) as StringLiteral).text); // _y + const uniqueName = factory.createUniqueName(localName); // _x const specifier = getSpecifierForModuleSymbol(target.parent || target, context); // 'y' - // import _y = require('y'); + // import _x = require('y'); addResult(factory.createImportEqualsDeclaration( /*decorators*/ undefined, /*modifiers*/ undefined, uniqueName, factory.createExternalModuleReference(factory.createStringLiteral(specifier)) ), ModifierFlags.None); - // import x = _y.z + // import x = _x.z addResult(factory.createImportEqualsDeclaration( /*decorators*/ undefined, /*modifiers*/ undefined, diff --git a/tests/baselines/reference/jsDeclarationsCommonjsRelativePath.js b/tests/baselines/reference/jsDeclarationsCommonjsRelativePath.js new file mode 100644 index 0000000000000..bf1b0bc7c601c --- /dev/null +++ b/tests/baselines/reference/jsDeclarationsCommonjsRelativePath.js @@ -0,0 +1,22 @@ +//// [tests/cases/conformance/jsdoc/declarations/jsDeclarationsCommonjsRelativePath.ts] //// + +//// [thing.js] +'use strict'; +class Thing {} +module.exports = { Thing } + +//// [reexport.js] +'use strict'; +const Thing = require('./thing').Thing +module.exports = { Thing } + + + + +//// [thing.d.ts] +export class Thing { +} +//// [reexport.d.ts] +import Thing_1 = require("./thing"); +import Thing = Thing_1.Thing; +export { Thing }; diff --git a/tests/baselines/reference/jsDeclarationsCommonjsRelativePath.symbols b/tests/baselines/reference/jsDeclarationsCommonjsRelativePath.symbols new file mode 100644 index 0000000000000..d8c0f63dfafa1 --- /dev/null +++ b/tests/baselines/reference/jsDeclarationsCommonjsRelativePath.symbols @@ -0,0 +1,26 @@ +=== tests/cases/conformance/jsdoc/declarations/reexport.js === +'use strict'; +const Thing = require('./thing').Thing +>Thing : Symbol(Thing, Decl(reexport.js, 1, 5)) +>require('./thing').Thing : Symbol(Thing, Decl(thing.js, 2, 18)) +>require : Symbol(require) +>'./thing' : Symbol("tests/cases/conformance/jsdoc/declarations/thing", Decl(thing.js, 0, 0)) +>Thing : Symbol(Thing, Decl(thing.js, 2, 18)) + +module.exports = { Thing } +>module.exports : Symbol("tests/cases/conformance/jsdoc/declarations/reexport", Decl(reexport.js, 0, 0)) +>module : Symbol(export=, Decl(reexport.js, 1, 38)) +>exports : Symbol(export=, Decl(reexport.js, 1, 38)) +>Thing : Symbol(Thing, Decl(reexport.js, 2, 18)) + +=== tests/cases/conformance/jsdoc/declarations/thing.js === +'use strict'; +class Thing {} +>Thing : Symbol(Thing, Decl(thing.js, 0, 13)) + +module.exports = { Thing } +>module.exports : Symbol("tests/cases/conformance/jsdoc/declarations/thing", Decl(thing.js, 0, 0)) +>module : Symbol(export=, Decl(thing.js, 1, 14)) +>exports : Symbol(export=, Decl(thing.js, 1, 14)) +>Thing : Symbol(Thing, Decl(thing.js, 2, 18)) + diff --git a/tests/baselines/reference/jsDeclarationsCommonjsRelativePath.types b/tests/baselines/reference/jsDeclarationsCommonjsRelativePath.types new file mode 100644 index 0000000000000..10856bd16ad1d --- /dev/null +++ b/tests/baselines/reference/jsDeclarationsCommonjsRelativePath.types @@ -0,0 +1,35 @@ +=== tests/cases/conformance/jsdoc/declarations/reexport.js === +'use strict'; +>'use strict' : "use strict" + +const Thing = require('./thing').Thing +>Thing : typeof Thing +>require('./thing').Thing : typeof Thing +>require('./thing') : { Thing: typeof Thing; } +>require : any +>'./thing' : "./thing" +>Thing : typeof Thing + +module.exports = { Thing } +>module.exports = { Thing } : { Thing: typeof Thing; } +>module.exports : { Thing: typeof Thing; } +>module : { "\"tests/cases/conformance/jsdoc/declarations/reexport\"": { Thing: typeof Thing; }; } +>exports : { Thing: typeof Thing; } +>{ Thing } : { Thing: typeof Thing; } +>Thing : typeof Thing + +=== tests/cases/conformance/jsdoc/declarations/thing.js === +'use strict'; +>'use strict' : "use strict" + +class Thing {} +>Thing : Thing + +module.exports = { Thing } +>module.exports = { Thing } : { Thing: typeof Thing; } +>module.exports : { Thing: typeof Thing; } +>module : { "\"tests/cases/conformance/jsdoc/declarations/thing\"": { Thing: typeof Thing; }; } +>exports : { Thing: typeof Thing; } +>{ Thing } : { Thing: typeof Thing; } +>Thing : typeof Thing + diff --git a/tests/baselines/reference/jsDeclarationsTypeReferences.js b/tests/baselines/reference/jsDeclarationsTypeReferences.js index d302fc8a5ef61..db5962f8cb207 100644 --- a/tests/baselines/reference/jsDeclarationsTypeReferences.js +++ b/tests/baselines/reference/jsDeclarationsTypeReferences.js @@ -28,5 +28,5 @@ module.exports = { //// [index.d.ts] /// export const thing: Something; -import fs_1 = require("fs"); -import Something = fs_1.Something; +import Something_1 = require("fs"); +import Something = Something_1.Something; diff --git a/tests/baselines/reference/jsDeclarationsTypeReferences3.js b/tests/baselines/reference/jsDeclarationsTypeReferences3.js index e765cc6164325..ea5f5a225512f 100644 --- a/tests/baselines/reference/jsDeclarationsTypeReferences3.js +++ b/tests/baselines/reference/jsDeclarationsTypeReferences3.js @@ -30,5 +30,5 @@ export namespace A { const thing: Something; } } -import fs_1 = require("fs"); -import Something = fs_1.Something; +import Something_1 = require("fs"); +import Something = Something_1.Something; diff --git a/tests/cases/conformance/jsdoc/declarations/jsDeclarationsCommonjsRelativePath.ts b/tests/cases/conformance/jsdoc/declarations/jsDeclarationsCommonjsRelativePath.ts new file mode 100644 index 0000000000000..cf7bc5eb597b1 --- /dev/null +++ b/tests/cases/conformance/jsdoc/declarations/jsDeclarationsCommonjsRelativePath.ts @@ -0,0 +1,13 @@ +// @checkJs: true +// @declaration: true +// @emitDeclarationOnly: true +// @module: commonjs +// @Filename: thing.js +'use strict'; +class Thing {} +module.exports = { Thing } + +// @Filename: reexport.js +'use strict'; +const Thing = require('./thing').Thing +module.exports = { Thing }