Skip to content

Commit a56b272

Browse files
authored
In JS, fix crash with in a file with conflicting ES2015/commonjs exports (microsoft#24960)
* fix crash with conflicting ES2015/commonjs modules * Refactor based on PR comments
1 parent a770688 commit a56b272

File tree

4 files changed

+65
-7
lines changed

4 files changed

+65
-7
lines changed

src/compiler/binder.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2305,18 +2305,22 @@ namespace ts {
23052305
}
23062306

23072307
function setCommonJsModuleIndicator(node: Node) {
2308+
if (file.externalModuleIndicator) {
2309+
return false;
2310+
}
23082311
if (!file.commonJsModuleIndicator) {
23092312
file.commonJsModuleIndicator = node;
2310-
if (!file.externalModuleIndicator) {
2311-
bindSourceFileAsExternalModule();
2312-
}
2313+
bindSourceFileAsExternalModule();
23132314
}
2315+
return true;
23142316
}
23152317

23162318
function bindExportsPropertyAssignment(node: BinaryExpression) {
23172319
// When we create a property via 'exports.foo = bar', the 'exports.foo' property access
23182320
// expression is the declaration
2319-
setCommonJsModuleIndicator(node);
2321+
if (!setCommonJsModuleIndicator(node)) {
2322+
return;
2323+
}
23202324
const lhs = node.left as PropertyAccessEntityNameExpression;
23212325
const symbol = forEachIdentifierInEntityName(lhs.expression, /*parent*/ undefined, (id, symbol) => {
23222326
if (symbol) {
@@ -2337,15 +2341,15 @@ namespace ts {
23372341
// is still pointing to 'module.exports'.
23382342
// We do not want to consider this as 'export=' since a module can have only one of these.
23392343
// Similarly we do not want to treat 'module.exports = exports' as an 'export='.
2344+
if (!setCommonJsModuleIndicator(node)) {
2345+
return;
2346+
}
23402347
const assignedExpression = getRightMostAssignedExpression(node.right);
23412348
if (isEmptyObjectLiteral(assignedExpression) || container === file && isExportsOrModuleExportsOrAlias(file, assignedExpression)) {
2342-
// Mark it as a module in case there are no other exports in the file
2343-
setCommonJsModuleIndicator(node);
23442349
return;
23452350
}
23462351

23472352
// 'module.exports = expr' assignment
2348-
setCommonJsModuleIndicator(node);
23492353
const flags = exportAssignmentIsAlias(node)
23502354
? SymbolFlags.Alias // An export= with an EntityNameExpression or a ClassExpression exports all meanings of that identifier or class
23512355
: SymbolFlags.Property | SymbolFlags.ExportValue | SymbolFlags.ValueModule;
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
=== tests/cases/conformance/salsa/bug24934.js ===
2+
export function abc(a, b, c) { return 5; }
3+
>abc : Symbol(abc, Decl(bug24934.js, 0, 0))
4+
>a : Symbol(a, Decl(bug24934.js, 0, 20))
5+
>b : Symbol(b, Decl(bug24934.js, 0, 22))
6+
>c : Symbol(c, Decl(bug24934.js, 0, 25))
7+
8+
module.exports = { abc };
9+
>module : Symbol(module)
10+
>abc : Symbol(abc, Decl(bug24934.js, 1, 18))
11+
12+
=== tests/cases/conformance/salsa/use.js ===
13+
import { abc } from './bug24934';
14+
>abc : Symbol(abc, Decl(use.js, 0, 8))
15+
16+
abc(1, 2, 3);
17+
>abc : Symbol(abc, Decl(use.js, 0, 8))
18+
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
=== tests/cases/conformance/salsa/bug24934.js ===
2+
export function abc(a, b, c) { return 5; }
3+
>abc : (a: any, b: any, c: any) => number
4+
>a : any
5+
>b : any
6+
>c : any
7+
>5 : 5
8+
9+
module.exports = { abc };
10+
>module.exports = { abc } : { [x: string]: any; abc: (a: any, b: any, c: any) => number; }
11+
>module.exports : any
12+
>module : any
13+
>exports : any
14+
>{ abc } : { [x: string]: any; abc: (a: any, b: any, c: any) => number; }
15+
>abc : (a: any, b: any, c: any) => number
16+
17+
=== tests/cases/conformance/salsa/use.js ===
18+
import { abc } from './bug24934';
19+
>abc : (a: any, b: any, c: any) => number
20+
21+
abc(1, 2, 3);
22+
>abc(1, 2, 3) : number
23+
>abc : (a: any, b: any, c: any) => number
24+
>1 : 1
25+
>2 : 2
26+
>3 : 3
27+
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// @checkJs: true
2+
// @allowJS: true
3+
// @noEmit: true
4+
// @Filename: bug24934.js
5+
export function abc(a, b, c) { return 5; }
6+
module.exports = { abc };
7+
// @Filename: use.js
8+
import { abc } from './bug24934';
9+
abc(1, 2, 3);

0 commit comments

Comments
 (0)