Skip to content

Commit ec36d73

Browse files
sandersnsandersn
authored
Fix error on duplicate commonjs exports (microsoft#40545)
* Fix error on duplicate commonjs exports Previously, the code missed setting the parent pointer for the lhs access expression. Also add declaration emit of element access expressions, missed in my previous PR. * Switch to excludes=None, add test case CommonJS exports have None excludes, but still have an error issued by the checker. This is the previous behaviour even though it would be nice to add some exclusions.
1 parent c493d07 commit ec36d73

11 files changed

+109
-3
lines changed

‎src/compiler/binder.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2810,8 +2810,8 @@ namespace ts {
28102810
if (symbol) {
28112811
const isAlias = isAliasableExpression(node.right) && (isExportsIdentifier(node.left.expression) || isModuleExportsAccessExpression(node.left.expression));
28122812
const flags = isAlias ? SymbolFlags.Alias : SymbolFlags.Property | SymbolFlags.ExportValue;
2813-
const excludeFlags = isAlias ? SymbolFlags.AliasExcludes : SymbolFlags.None;
2814-
declareSymbol(symbol.exports!, symbol, node.left, flags, excludeFlags);
2813+
setParent(node.left, node);
2814+
declareSymbol(symbol.exports!, symbol, node.left, flags, SymbolFlags.None);
28152815
}
28162816
}
28172817

‎src/compiler/checker.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6831,6 +6831,7 @@ namespace ts {
68316831
break;
68326832
case SyntaxKind.BinaryExpression:
68336833
case SyntaxKind.PropertyAccessExpression:
6834+
case SyntaxKind.ElementAccessExpression:
68346835
// Could be best encoded as though an export specifier or as though an export assignment
68356836
// If name is default or export=, do an export assignment
68366837
// Otherwise do an export specifier

‎tests/baselines/reference/moduleExportAliasElementAccessExpression.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//// [moduleExportAliasElementAccessExpression.js]
2+
function D () { }
3+
exports["D"] = D;
4+
// (the only package I could find that uses spaces in identifiers is webidl-conversions)
5+
exports["Does not work yet"] = D;
6+
7+
8+
//// [moduleExportAliasElementAccessExpression.js]
9+
function D() { }
10+
exports["D"] = D;
11+
// (the only package I could find that uses spaces in identifiers is webidl-conversions)
12+
exports["Does not work yet"] = D;
13+
14+
15+
//// [moduleExportAliasElementAccessExpression.d.ts]
16+
export function D(): void;
17+
export { D as _Does_not_work_yet };

‎tests/baselines/reference/moduleExportAliasElementAccessExpression.symbols‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,9 @@ exports["D"] = D;
77
>"D" : Symbol("D", Decl(moduleExportAliasElementAccessExpression.js, 0, 17))
88
>D : Symbol(D, Decl(moduleExportAliasElementAccessExpression.js, 0, 0))
99

10+
// (the only package I could find that uses spaces in identifiers is webidl-conversions)
11+
exports["Does not work yet"] = D;
12+
>exports : Symbol("tests/cases/conformance/salsa/moduleExportAliasElementAccessExpression", Decl(moduleExportAliasElementAccessExpression.js, 0, 0))
13+
>"Does not work yet" : Symbol("Does not work yet", Decl(moduleExportAliasElementAccessExpression.js, 1, 17))
14+
>D : Symbol(D, Decl(moduleExportAliasElementAccessExpression.js, 0, 0))
15+

‎tests/baselines/reference/moduleExportAliasElementAccessExpression.types‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,11 @@ exports["D"] = D;
99
>"D" : "D"
1010
>D : () => void
1111

12+
// (the only package I could find that uses spaces in identifiers is webidl-conversions)
13+
exports["Does not work yet"] = D;
14+
>exports["Does not work yet"] = D : () => void
15+
>exports["Does not work yet"] : () => void
16+
>exports : typeof import("tests/cases/conformance/salsa/moduleExportAliasElementAccessExpression")
17+
>"Does not work yet" : "Does not work yet"
18+
>D : () => void
19+

‎tests/baselines/reference/moduleExportDuplicateAlias.errors.txt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js(1,1): error TS2323: Cannot redeclare exported variable 'apply'.
2+
tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js(3,1): error TS2322: Type '() => void' is not assignable to type 'undefined'.
3+
tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js(3,1): error TS2323: Cannot redeclare exported variable 'apply'.
4+
5+
6+
==== tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js (3 errors) ====
7+
exports.apply = undefined;
8+
~~~~~~~~~~~~~
9+
!!! error TS2323: Cannot redeclare exported variable 'apply'.
10+
function a() { }
11+
exports.apply = a;
12+
~~~~~~~~~~~~~
13+
!!! error TS2322: Type '() => void' is not assignable to type 'undefined'.
14+
~~~~~~~~~~~~~
15+
!!! error TS2323: Cannot redeclare exported variable 'apply'.
16+

‎tests/baselines/reference/moduleExportDuplicateAlias.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//// [moduleExportAliasDuplicateAlias.js]
2+
exports.apply = undefined;
3+
function a() { }
4+
exports.apply = a;
5+
6+
7+
//// [moduleExportAliasDuplicateAlias.js]
8+
exports.apply = undefined;
9+
function a() { }
10+
exports.apply = a;
11+
12+
13+
//// [moduleExportAliasDuplicateAlias.d.ts]
14+
export { undefined as apply };

‎tests/baselines/reference/moduleExportDuplicateAlias.symbols‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
=== tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js ===
2+
exports.apply = undefined;
3+
>exports.apply : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16))
4+
>exports : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16))
5+
>apply : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16))
6+
>undefined : Symbol(apply)
7+
8+
function a() { }
9+
>a : Symbol(a, Decl(moduleExportAliasDuplicateAlias.js, 0, 26))
10+
11+
exports.apply = a;
12+
>exports.apply : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16))
13+
>exports : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16))
14+
>apply : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16))
15+
>a : Symbol(a, Decl(moduleExportAliasDuplicateAlias.js, 0, 26))
16+

‎tests/baselines/reference/moduleExportDuplicateAlias.types‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
=== tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js ===
2+
exports.apply = undefined;
3+
>exports.apply = undefined : undefined
4+
>exports.apply : undefined
5+
>exports : typeof import("tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias")
6+
>apply : undefined
7+
>undefined : undefined
8+
9+
function a() { }
10+
>a : () => void
11+
12+
exports.apply = a;
13+
>exports.apply = a : () => void
14+
>exports.apply : undefined
15+
>exports : typeof import("tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias")
16+
>apply : undefined
17+
>a : () => void
18+

‎tests/cases/conformance/salsa/moduleExportAliasElementAccessExpression.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1-
// @noEmit: true
1+
// @declaration: true
22
// @checkJs: true
33
// @filename: moduleExportAliasElementAccessExpression.js
4+
// @outdir: out
45

56
function D () { }
67
exports["D"] = D;
8+
// (the only package I could find that uses spaces in identifiers is webidl-conversions)
9+
exports["Does not work yet"] = D;

0 commit comments

Comments
 (0)