Skip to content

Commit

Permalink
Add missed resolveSymbol in commonjs import resolution (#41479) (#41691)
Browse files Browse the repository at this point in the history
Fixes resolution of export aliases in the postfix-property-access case
of commonjs require:

```js
const { x } = require('./foo').nested
x
```

This program would previously fail if `x` was an export alias.

Fixes #41422

Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
  • Loading branch information
andrewbranch and sandersn committed Dec 2, 2020
1 parent 9d25e59 commit 9b66258
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
Expand Up @@ -2800,7 +2800,7 @@ namespace ts {
const resolved = getExternalModuleMember(root, commonJSPropertyAccess || node, dontResolveAlias);
const name = node.propertyName || node.name;
if (commonJSPropertyAccess && resolved && isIdentifier(name)) {
return getPropertyOfType(getTypeOfSymbol(resolved), name.escapedText);
return resolveSymbol(getPropertyOfType(getTypeOfSymbol(resolved), name.escapedText), dontResolveAlias);
}
markSymbolOfAliasDeclarationIfTypeOnly(node, /*immediateTarget*/ undefined, resolved, /*overwriteEmpty*/ false);
return resolved;
Expand Down
41 changes: 41 additions & 0 deletions tests/baselines/reference/commonJSReexport.symbols
@@ -0,0 +1,41 @@
=== tests/cases/conformance/salsa/main.js ===
const { hardline } = require('./second').nested;
>hardline : Symbol(hardline, Decl(main.js, 0, 7))
>require('./second').nested : Symbol(nested, Decl(second.js, 0, 18))
>require : Symbol(require)
>'./second' : Symbol("tests/cases/conformance/salsa/second", Decl(second.js, 0, 0))
>nested : Symbol(nested, Decl(second.js, 0, 18))

hardline
>hardline : Symbol(hardline, Decl(main.js, 0, 7))

=== tests/cases/conformance/salsa/first.js ===
// #41422, based on prettier's exports

const hardline = { type: "hard" }
>hardline : Symbol(hardline, Decl(first.js, 2, 5))
>type : Symbol(type, Decl(first.js, 2, 18))

module.exports = {
>module.exports : Symbol("tests/cases/conformance/salsa/first", Decl(first.js, 0, 0))
>module : Symbol(module, Decl(first.js, 2, 33))
>exports : Symbol("tests/cases/conformance/salsa/first", Decl(first.js, 0, 0))

hardline
>hardline : Symbol(hardline, Decl(first.js, 3, 18))
}


=== tests/cases/conformance/salsa/second.js ===
module.exports = {
>module.exports : Symbol("tests/cases/conformance/salsa/second", Decl(second.js, 0, 0))
>module : Symbol(export=, Decl(second.js, 0, 0))
>exports : Symbol(export=, Decl(second.js, 0, 0))

nested: require('./first')
>nested : Symbol(nested, Decl(second.js, 0, 18))
>require : Symbol(require)
>'./first' : Symbol("tests/cases/conformance/salsa/first", Decl(first.js, 0, 0))

};

49 changes: 49 additions & 0 deletions tests/baselines/reference/commonJSReexport.types
@@ -0,0 +1,49 @@
=== tests/cases/conformance/salsa/main.js ===
const { hardline } = require('./second').nested;
>hardline : { type: string; }
>require('./second').nested : typeof import("tests/cases/conformance/salsa/first")
>require('./second') : { nested: typeof import("tests/cases/conformance/salsa/first"); }
>require : any
>'./second' : "./second"
>nested : typeof import("tests/cases/conformance/salsa/first")

hardline
>hardline : { type: string; }

=== tests/cases/conformance/salsa/first.js ===
// #41422, based on prettier's exports

const hardline = { type: "hard" }
>hardline : { type: string; }
>{ type: "hard" } : { type: string; }
>type : string
>"hard" : "hard"

module.exports = {
>module.exports = { hardline} : typeof import("tests/cases/conformance/salsa/first")
>module.exports : typeof import("tests/cases/conformance/salsa/first")
>module : { "\"tests/cases/conformance/salsa/first\"": typeof import("tests/cases/conformance/salsa/first"); }
>exports : typeof import("tests/cases/conformance/salsa/first")
>{ hardline} : { hardline: { type: string; }; }

hardline
>hardline : { type: string; }
}


=== tests/cases/conformance/salsa/second.js ===
module.exports = {
>module.exports = { nested: require('./first')} : { nested: typeof import("tests/cases/conformance/salsa/first"); }
>module.exports : { nested: typeof import("tests/cases/conformance/salsa/first"); }
>module : { "\"tests/cases/conformance/salsa/second\"": { nested: typeof import("tests/cases/conformance/salsa/first"); }; }
>exports : { nested: typeof import("tests/cases/conformance/salsa/first"); }
>{ nested: require('./first')} : { nested: typeof import("tests/cases/conformance/salsa/first"); }

nested: require('./first')
>nested : typeof import("tests/cases/conformance/salsa/first")
>require('./first') : typeof import("tests/cases/conformance/salsa/first")
>require : any
>'./first' : "./first"

};

19 changes: 19 additions & 0 deletions tests/cases/conformance/salsa/commonJSReexport.ts
@@ -0,0 +1,19 @@
// #41422, based on prettier's exports
// @noEmit: true
// @checkJS: true

// @filename: first.js
const hardline = { type: "hard" }
module.exports = {
hardline
}


// @filename: second.js
module.exports = {
nested: require('./first')
};

// @filename: main.js
const { hardline } = require('./second').nested;
hardline

0 comments on commit 9b66258

Please sign in to comment.