Skip to content

Commit

Permalink
Fix relative paths in commonjs decl emit w/property access (#40986)
Browse files Browse the repository at this point in the history
```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
  • Loading branch information
sandersn committed Oct 9, 2020
1 parent aee18e0 commit a109b5d
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 7 deletions.
6 changes: 3 additions & 3 deletions src/compiler/checker.ts
Expand Up @@ -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,
Expand Down
22 changes: 22 additions & 0 deletions 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 };
@@ -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))

35 changes: 35 additions & 0 deletions 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

4 changes: 2 additions & 2 deletions tests/baselines/reference/jsDeclarationsTypeReferences.js
Expand Up @@ -28,5 +28,5 @@ module.exports = {
//// [index.d.ts]
/// <reference types="node" />
export const thing: Something;
import fs_1 = require("fs");
import Something = fs_1.Something;
import Something_1 = require("fs");
import Something = Something_1.Something;
4 changes: 2 additions & 2 deletions tests/baselines/reference/jsDeclarationsTypeReferences3.js
Expand Up @@ -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;
@@ -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 }

0 comments on commit a109b5d

Please sign in to comment.