Skip to content

Commit a7a01ea

Browse files
authored
Allow both module.exports= and module.exports property assignments (#23228)
* Combining symbol and removing error done but messy * Small fix + add new test baselines All other tests are unchanged * Union conflicting assignment types+better names * Add tests and update baselines * Check commonjs export= from resolveExternalModuleSymbol
1 parent 70682b7 commit a7a01ea

17 files changed

+781
-4
lines changed

src/compiler/checker.ts

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2216,7 +2216,22 @@ namespace ts {
22162216
// An external module with an 'export =' declaration resolves to the target of the 'export =' declaration,
22172217
// and an external module with no 'export =' declaration resolves to the module itself.
22182218
function resolveExternalModuleSymbol(moduleSymbol: Symbol, dontResolveAlias?: boolean): Symbol {
2219-
return moduleSymbol && getMergedSymbol(resolveSymbol(moduleSymbol.exports.get(InternalSymbolName.ExportEquals), dontResolveAlias)) || moduleSymbol;
2219+
return moduleSymbol && getMergedSymbol(resolveSymbol(getCommonJsExportEquals(moduleSymbol), dontResolveAlias)) || moduleSymbol;
2220+
}
2221+
2222+
function getCommonJsExportEquals(moduleSymbol: Symbol): Symbol {
2223+
const exported = moduleSymbol.exports.get(InternalSymbolName.ExportEquals);
2224+
if (!exported || !exported.exports || moduleSymbol.exports.size === 1) {
2225+
return exported;
2226+
}
2227+
const merged = cloneSymbol(exported);
2228+
moduleSymbol.exports.forEach((s, name) => {
2229+
if (name === InternalSymbolName.ExportEquals) return;
2230+
if (!merged.exports.has(name)) {
2231+
merged.exports.set(name, s);
2232+
}
2233+
});
2234+
return merged;
22202235
}
22212236

22222237
// An external module with an 'export =' declaration may be referenced as an ES6 module provided the 'export ='
@@ -4350,7 +4365,8 @@ namespace ts {
43504365
return unknownType;
43514366
}
43524367

4353-
if (isPropertyAccessExpression(expression.left) && expression.left.expression.kind === SyntaxKind.ThisKeyword) {
4368+
const special = getSpecialPropertyAssignmentKind(expression);
4369+
if (special === SpecialPropertyAssignmentKind.ThisProperty) {
43544370
const thisContainer = getThisContainer(expression, /*includeArrowFunctions*/ false);
43554371
// Properties defined in a constructor (or javascript constructor function) don't get undefined added.
43564372
// Function expressions that are assigned to the prototype count as methods.
@@ -4380,7 +4396,33 @@ namespace ts {
43804396
}
43814397
else if (!jsDocType) {
43824398
// If we don't have an explicit JSDoc type, get the type from the expression.
4383-
const type = getWidenedLiteralType(checkExpressionCached(expression.right));
4399+
let type = getWidenedLiteralType(checkExpressionCached(expression.right));
4400+
4401+
if (getObjectFlags(type) & ObjectFlags.Anonymous &&
4402+
special === SpecialPropertyAssignmentKind.ModuleExports &&
4403+
symbol.escapedName === InternalSymbolName.ExportEquals) {
4404+
const exportedType = resolveStructuredTypeMembers(type as AnonymousType);
4405+
const members = createSymbolTable();
4406+
copyEntries(exportedType.members, members);
4407+
symbol.exports.forEach((s, name) => {
4408+
if (members.has(name)) {
4409+
const exportedMember = exportedType.members.get(name);
4410+
const union = createSymbol(s.flags | exportedMember.flags, name);
4411+
union.type = getUnionType([getTypeOfSymbol(s), getTypeOfSymbol(exportedMember)]);
4412+
members.set(name, union);
4413+
}
4414+
else {
4415+
members.set(name, s);
4416+
}
4417+
});
4418+
type = createAnonymousType(
4419+
exportedType.symbol,
4420+
members,
4421+
exportedType.callSignatures,
4422+
exportedType.constructSignatures,
4423+
exportedType.stringIndexInfo,
4424+
exportedType.numberIndexInfo);
4425+
}
43844426
let anyedType = type;
43854427
if (isEmptyArrayLiteralType(type)) {
43864428
anyedType = anyArrayType;
@@ -24574,7 +24616,7 @@ namespace ts {
2457424616
const exportEqualsSymbol = moduleSymbol.exports.get("export=" as __String);
2457524617
if (exportEqualsSymbol && hasExportedMembers(moduleSymbol)) {
2457624618
const declaration = getDeclarationOfAliasSymbol(exportEqualsSymbol) || exportEqualsSymbol.valueDeclaration;
24577-
if (!isTopLevelInExternalModuleAugmentation(declaration)) {
24619+
if (!isTopLevelInExternalModuleAugmentation(declaration) && !isInJavaScriptFile(declaration)) {
2457824620
error(declaration, Diagnostics.An_export_assignment_cannot_be_used_in_a_module_with_other_exported_elements);
2457924621
}
2458024622
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
tests/cases/conformance/salsa/a.js(4,1): error TS2554: Expected 1 arguments, but got 0.
2+
3+
4+
==== tests/cases/conformance/salsa/a.js (1 errors) ====
5+
/// <reference path='./requires.d.ts' />
6+
var mod1 = require('./mod1')
7+
mod1()
8+
mod1.f() // error, not enough arguments
9+
~~~~~~~~
10+
!!! error TS2554: Expected 1 arguments, but got 0.
11+
12+
==== tests/cases/conformance/salsa/requires.d.ts (0 errors) ====
13+
declare var module: { exports: any };
14+
declare function require(name: string): any;
15+
==== tests/cases/conformance/salsa/mod1.js (0 errors) ====
16+
/// <reference path='./requires.d.ts' />
17+
module.exports = function () { }
18+
/** @param {number} a */
19+
module.exports.f = function (a) { }
20+
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
/// <reference path='./requires.d.ts' />
3+
var mod1 = require('./mod1')
4+
>mod1 : Symbol(mod1, Decl(a.js, 1, 3))
5+
>require : Symbol(require, Decl(requires.d.ts, 0, 37))
6+
>'./mod1' : Symbol("tests/cases/conformance/salsa/mod1", Decl(mod1.js, 0, 0))
7+
8+
mod1()
9+
>mod1 : Symbol(mod1, Decl(a.js, 1, 3))
10+
11+
mod1.f() // error, not enough arguments
12+
>mod1.f : Symbol(f, Decl(mod1.js, 1, 32))
13+
>mod1 : Symbol(mod1, Decl(a.js, 1, 3))
14+
>f : Symbol(f, Decl(mod1.js, 1, 32))
15+
16+
=== tests/cases/conformance/salsa/requires.d.ts ===
17+
declare var module: { exports: any };
18+
>module : Symbol(module, Decl(requires.d.ts, 0, 11))
19+
>exports : Symbol(exports, Decl(requires.d.ts, 0, 21))
20+
21+
declare function require(name: string): any;
22+
>require : Symbol(require, Decl(requires.d.ts, 0, 37))
23+
>name : Symbol(name, Decl(requires.d.ts, 1, 25))
24+
25+
=== tests/cases/conformance/salsa/mod1.js ===
26+
/// <reference path='./requires.d.ts' />
27+
module.exports = function () { }
28+
>module.exports : Symbol(exports, Decl(requires.d.ts, 0, 21))
29+
>module : Symbol(export=, Decl(mod1.js, 0, 0))
30+
>exports : Symbol(export=, Decl(mod1.js, 0, 0))
31+
32+
/** @param {number} a */
33+
module.exports.f = function (a) { }
34+
>module.exports : Symbol(f, Decl(mod1.js, 1, 32))
35+
>module : Symbol(module, Decl(requires.d.ts, 0, 11))
36+
>exports : Symbol(exports, Decl(requires.d.ts, 0, 21))
37+
>f : Symbol(f, Decl(mod1.js, 1, 32))
38+
>a : Symbol(a, Decl(mod1.js, 3, 29))
39+
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
/// <reference path='./requires.d.ts' />
3+
var mod1 = require('./mod1')
4+
>mod1 : { (): void; f: (a: number) => void; }
5+
>require('./mod1') : { (): void; f: (a: number) => void; }
6+
>require : (name: string) => any
7+
>'./mod1' : "./mod1"
8+
9+
mod1()
10+
>mod1() : void
11+
>mod1 : { (): void; f: (a: number) => void; }
12+
13+
mod1.f() // error, not enough arguments
14+
>mod1.f() : void
15+
>mod1.f : (a: number) => void
16+
>mod1 : { (): void; f: (a: number) => void; }
17+
>f : (a: number) => void
18+
19+
=== tests/cases/conformance/salsa/requires.d.ts ===
20+
declare var module: { exports: any };
21+
>module : { exports: any; }
22+
>exports : any
23+
24+
declare function require(name: string): any;
25+
>require : (name: string) => any
26+
>name : string
27+
28+
=== tests/cases/conformance/salsa/mod1.js ===
29+
/// <reference path='./requires.d.ts' />
30+
module.exports = function () { }
31+
>module.exports = function () { } : () => void
32+
>module.exports : any
33+
>module : { exports: any; }
34+
>exports : any
35+
>function () { } : () => void
36+
37+
/** @param {number} a */
38+
module.exports.f = function (a) { }
39+
>module.exports.f = function (a) { } : (a: number) => void
40+
>module.exports.f : any
41+
>module.exports : any
42+
>module : { exports: any; }
43+
>exports : any
44+
>f : any
45+
>function (a) { } : (a: number) => void
46+
>a : number
47+
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
tests/cases/conformance/salsa/a.js(4,6): error TS2339: Property 'f' does not exist on type 'number'.
2+
3+
4+
==== tests/cases/conformance/salsa/a.js (1 errors) ====
5+
/// <reference path='./requires.d.ts' />
6+
var mod1 = require('./mod1')
7+
mod1.toFixed(12)
8+
mod1.f() // error, 'f' is not a property on 'number'
9+
~
10+
!!! error TS2339: Property 'f' does not exist on type 'number'.
11+
12+
==== tests/cases/conformance/salsa/requires.d.ts (0 errors) ====
13+
declare var module: { exports: any };
14+
declare function require(name: string): any;
15+
==== tests/cases/conformance/salsa/mod1.js (0 errors) ====
16+
/// <reference path='./requires.d.ts' />
17+
module.exports = 1
18+
module.exports.f = function () { }
19+
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
/// <reference path='./requires.d.ts' />
3+
var mod1 = require('./mod1')
4+
>mod1 : Symbol(mod1, Decl(a.js, 1, 3))
5+
>require : Symbol(require, Decl(requires.d.ts, 0, 37))
6+
>'./mod1' : Symbol("tests/cases/conformance/salsa/mod1", Decl(mod1.js, 0, 0))
7+
8+
mod1.toFixed(12)
9+
>mod1.toFixed : Symbol(Number.toFixed, Decl(lib.d.ts, --, --))
10+
>mod1 : Symbol(mod1, Decl(a.js, 1, 3))
11+
>toFixed : Symbol(Number.toFixed, Decl(lib.d.ts, --, --))
12+
13+
mod1.f() // error, 'f' is not a property on 'number'
14+
>mod1 : Symbol(mod1, Decl(a.js, 1, 3))
15+
16+
=== tests/cases/conformance/salsa/requires.d.ts ===
17+
declare var module: { exports: any };
18+
>module : Symbol(module, Decl(requires.d.ts, 0, 11))
19+
>exports : Symbol(exports, Decl(requires.d.ts, 0, 21))
20+
21+
declare function require(name: string): any;
22+
>require : Symbol(require, Decl(requires.d.ts, 0, 37))
23+
>name : Symbol(name, Decl(requires.d.ts, 1, 25))
24+
25+
=== tests/cases/conformance/salsa/mod1.js ===
26+
/// <reference path='./requires.d.ts' />
27+
module.exports = 1
28+
>module.exports : Symbol(exports, Decl(requires.d.ts, 0, 21))
29+
>module : Symbol(export=, Decl(mod1.js, 0, 0))
30+
>exports : Symbol(export=, Decl(mod1.js, 0, 0))
31+
32+
module.exports.f = function () { }
33+
>module.exports : Symbol(f, Decl(mod1.js, 1, 18))
34+
>module : Symbol(module, Decl(requires.d.ts, 0, 11))
35+
>exports : Symbol(exports, Decl(requires.d.ts, 0, 21))
36+
>f : Symbol(f, Decl(mod1.js, 1, 18))
37+
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
/// <reference path='./requires.d.ts' />
3+
var mod1 = require('./mod1')
4+
>mod1 : number
5+
>require('./mod1') : number
6+
>require : (name: string) => any
7+
>'./mod1' : "./mod1"
8+
9+
mod1.toFixed(12)
10+
>mod1.toFixed(12) : string
11+
>mod1.toFixed : (fractionDigits?: number) => string
12+
>mod1 : number
13+
>toFixed : (fractionDigits?: number) => string
14+
>12 : 12
15+
16+
mod1.f() // error, 'f' is not a property on 'number'
17+
>mod1.f() : any
18+
>mod1.f : any
19+
>mod1 : number
20+
>f : any
21+
22+
=== tests/cases/conformance/salsa/requires.d.ts ===
23+
declare var module: { exports: any };
24+
>module : { exports: any; }
25+
>exports : any
26+
27+
declare function require(name: string): any;
28+
>require : (name: string) => any
29+
>name : string
30+
31+
=== tests/cases/conformance/salsa/mod1.js ===
32+
/// <reference path='./requires.d.ts' />
33+
module.exports = 1
34+
>module.exports = 1 : 1
35+
>module.exports : any
36+
>module : { exports: any; }
37+
>exports : any
38+
>1 : 1
39+
40+
module.exports.f = function () { }
41+
>module.exports.f = function () { } : () => void
42+
>module.exports.f : any
43+
>module.exports : any
44+
>module : { exports: any; }
45+
>exports : any
46+
>f : any
47+
>function () { } : () => void
48+
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
tests/cases/conformance/salsa/a.js(4,17): error TS2339: Property 'toFixed' does not exist on type 'string | number'.
2+
Property 'toFixed' does not exist on type 'string'.
3+
tests/cases/conformance/salsa/a.js(5,16): error TS2339: Property 'toFixed' does not exist on type 'string | number'.
4+
Property 'toFixed' does not exist on type 'string'.
5+
6+
7+
==== tests/cases/conformance/salsa/a.js (2 errors) ====
8+
/// <reference path='./requires.d.ts' />
9+
var mod1 = require('./mod1')
10+
mod1.justExport.toFixed()
11+
mod1.bothBefore.toFixed() // error, 'toFixed' not on 'string | number'
12+
~~~~~~~
13+
!!! error TS2339: Property 'toFixed' does not exist on type 'string | number'.
14+
!!! error TS2339: Property 'toFixed' does not exist on type 'string'.
15+
mod1.bothAfter.toFixed() // error, 'toFixed' not on 'string | number'
16+
~~~~~~~
17+
!!! error TS2339: Property 'toFixed' does not exist on type 'string | number'.
18+
!!! error TS2339: Property 'toFixed' does not exist on type 'string'.
19+
mod1.justProperty.length
20+
21+
==== tests/cases/conformance/salsa/requires.d.ts (0 errors) ====
22+
declare var module: { exports: any };
23+
declare function require(name: string): any;
24+
==== tests/cases/conformance/salsa/mod1.js (0 errors) ====
25+
/// <reference path='./requires.d.ts' />
26+
module.exports.bothBefore = 'string'
27+
module.exports = {
28+
justExport: 1,
29+
bothBefore: 2,
30+
bothAfter: 3,
31+
}
32+
module.exports.bothAfter = 'string'
33+
module.exports.justProperty = 'string'
34+

0 commit comments

Comments
 (0)