Skip to content

Commit 427d436

Browse files
sandersnsandersn
authored
Improve import type support for commonjs exports (microsoft#49745)
* Improve import type support for commonjs exports This PR makes getTypeFromImportTypeNode a little more like getExternalModuleMember: for JS files, it now uses both `getTypeOfSymbol` and `getExportsOfSymbol`, and uses whichever one returns a symbol. This allows using arbitrary properties of a CJS export= as types in JSDoc; previously a special case in the binder enabled only CJS export= where all properties were shorthand assignments. Fixes microsoft#49195 * Add js types/value test case * Improve binding of CJS property assignments 1. Bind property assignments same as shorthand property assignments in module.exports object literal assignments. 2. Bind all such assignments, even if the object literal contains non-property assignments. This is different from before, and it requires slightly smarter code to prefer aliases when checking CJS imports. * Remove new binder code Just include the original fix * revert missed type in binder
1 parent e1ceb2e commit 427d436

File tree

5 files changed

+612
-9
lines changed

5 files changed

+612
-9
lines changed

‎src/compiler/checker.ts

+7-9
Original file line numberDiff line numberDiff line change
@@ -3099,11 +3099,6 @@ namespace ts {
30993099
return getNodeLinks(expression).resolvedSymbol;
31003100
}
31013101

3102-
function getTargetOfPropertyAssignment(node: PropertyAssignment, dontRecursivelyResolve: boolean): Symbol | undefined {
3103-
const expression = node.initializer;
3104-
return getTargetOfAliasLikeExpression(expression, dontRecursivelyResolve);
3105-
}
3106-
31073102
function getTargetOfAccessExpression(node: AccessExpression, dontRecursivelyResolve: boolean): Symbol | undefined {
31083103
if (!(isBinaryExpression(node.parent) && node.parent.left === node && node.parent.operatorToken.kind === SyntaxKind.EqualsToken)) {
31093104
return undefined;
@@ -3136,7 +3131,7 @@ namespace ts {
31363131
case SyntaxKind.ShorthandPropertyAssignment:
31373132
return resolveEntityName((node as ShorthandPropertyAssignment).name, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace, /*ignoreErrors*/ true, dontRecursivelyResolve);
31383133
case SyntaxKind.PropertyAssignment:
3139-
return getTargetOfPropertyAssignment(node as PropertyAssignment, dontRecursivelyResolve);
3134+
return getTargetOfAliasLikeExpression((node as PropertyAssignment).initializer, dontRecursivelyResolve);
31403135
case SyntaxKind.ElementAccessExpression:
31413136
case SyntaxKind.PropertyAccessExpression:
31423137
return getTargetOfAccessExpression(node as AccessExpression, dontRecursivelyResolve);
@@ -9353,7 +9348,7 @@ namespace ts {
93539348
}
93549349
(resolvedSymbol || symbol).exports!.forEach((s, name) => {
93559350
const exportedMember = members.get(name)!;
9356-
if (exportedMember && exportedMember !== s) {
9351+
if (exportedMember && exportedMember !== s && !(s.flags & SymbolFlags.Alias)) {
93579352
if (s.flags & SymbolFlags.Value && exportedMember.flags & SymbolFlags.Value) {
93589353
// If the member has an additional value-like declaration, union the types from the two declarations,
93599354
// but issue an error if they occurred in two different files. The purpose is to support a JS file with
@@ -16298,6 +16293,7 @@ namespace ts {
1629816293
links.resolvedSymbol = unknownSymbol;
1629916294
return links.resolvedType = errorType;
1630016295
}
16296+
const isExportEquals = !!innerModuleSymbol.exports?.get(InternalSymbolName.ExportEquals);
1630116297
const moduleSymbol = resolveExternalModuleSymbol(innerModuleSymbol, /*dontResolveAlias*/ false);
1630216298
if (!nodeIsMissing(node.qualifier)) {
1630316299
const nameStack: Identifier[] = getIdentifierChain(node.qualifier!);
@@ -16309,9 +16305,11 @@ namespace ts {
1630916305
// That, in turn, ultimately uses `getPropertyOfType` on the type of the symbol, which differs slightly from
1631016306
// the `exports` lookup process that only looks up namespace members which is used for most type references
1631116307
const mergedResolvedSymbol = getMergedSymbol(resolveSymbol(currentNamespace));
16312-
const next = node.isTypeOf
16308+
const symbolFromVariable = node.isTypeOf || isInJSFile(node) && isExportEquals
1631316309
? getPropertyOfType(getTypeOfSymbol(mergedResolvedSymbol), current.escapedText, /*skipObjectFunctionPropertyAugment*/ false, /*includeTypeOnlyMembers*/ true)
16314-
: getSymbol(getExportsOfSymbol(mergedResolvedSymbol), current.escapedText, meaning);
16310+
: undefined;
16311+
const symbolFromModule = node.isTypeOf ? undefined : getSymbol(getExportsOfSymbol(mergedResolvedSymbol), current.escapedText, meaning);
16312+
const next = symbolFromModule ?? symbolFromVariable;
1631516313
if (!next) {
1631616314
error(current, Diagnostics.Namespace_0_has_no_exported_member_1, getFullyQualifiedName(currentNamespace), declarationNameToString(current));
1631716315
return links.resolvedType = errorType;

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

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
tests/cases/conformance/salsa/index.ts(2,24): error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'Thing'.
2+
tests/cases/conformance/salsa/index.ts(3,24): error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'AnotherThing'.
3+
tests/cases/conformance/salsa/index.ts(4,24): error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'foo'.
4+
tests/cases/conformance/salsa/index.ts(5,24): error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'qux'.
5+
tests/cases/conformance/salsa/index.ts(6,24): error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'baz'.
6+
tests/cases/conformance/salsa/index.ts(8,24): error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'literal'.
7+
tests/cases/conformance/salsa/index.ts(19,31): error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'buz'.
8+
tests/cases/conformance/salsa/main.js(20,35): error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'buz'.
9+
10+
11+
==== tests/cases/conformance/salsa/mod.js (0 errors) ====
12+
class Thing { x = 1 }
13+
class AnotherThing { y = 2 }
14+
function foo() { return 3 }
15+
function bar() { return 4 }
16+
/** @typedef {() => number} buz */
17+
module.exports = {
18+
Thing,
19+
AnotherThing,
20+
foo,
21+
qux: bar,
22+
baz() { return 5 },
23+
literal: "",
24+
}
25+
==== tests/cases/conformance/salsa/main.js (1 errors) ====
26+
/**
27+
* @param {import("./mod").Thing} a
28+
* @param {import("./mod").AnotherThing} b
29+
* @param {import("./mod").foo} c
30+
* @param {import("./mod").qux} d
31+
* @param {import("./mod").baz} e
32+
* @param {import("./mod").buz} f
33+
* @param {import("./mod").literal} g
34+
*/
35+
function jstypes(a, b, c, d, e, f, g) {
36+
return a.x + b.y + c() + d() + e() + f() + g.length
37+
}
38+
39+
/**
40+
* @param {typeof import("./mod").Thing} a
41+
* @param {typeof import("./mod").AnotherThing} b
42+
* @param {typeof import("./mod").foo} c
43+
* @param {typeof import("./mod").qux} d
44+
* @param {typeof import("./mod").baz} e
45+
* @param {typeof import("./mod").buz} f
46+
~~~
47+
!!! error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'buz'.
48+
* @param {typeof import("./mod").literal} g
49+
*/
50+
function jsvalues(a, b, c, d, e, f, g) {
51+
return a.length + b.length + c() + d() + e() + f() + g.length
52+
}
53+
54+
==== tests/cases/conformance/salsa/index.ts (7 errors) ====
55+
function types(
56+
a: import('./mod').Thing,
57+
~~~~~
58+
!!! error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'Thing'.
59+
b: import('./mod').AnotherThing,
60+
~~~~~~~~~~~~
61+
!!! error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'AnotherThing'.
62+
c: import('./mod').foo,
63+
~~~
64+
!!! error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'foo'.
65+
d: import('./mod').qux,
66+
~~~
67+
!!! error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'qux'.
68+
e: import('./mod').baz,
69+
~~~
70+
!!! error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'baz'.
71+
f: import('./mod').buz,
72+
g: import('./mod').literal,
73+
~~~~~~~
74+
!!! error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'literal'.
75+
) {
76+
return a.x + b.y + c() + d() + e() + f() + g.length
77+
}
78+
79+
function values(
80+
a: typeof import('./mod').Thing,
81+
b: typeof import('./mod').AnotherThing,
82+
c: typeof import('./mod').foo,
83+
d: typeof import('./mod').qux,
84+
e: typeof import('./mod').baz,
85+
f: typeof import('./mod').buz,
86+
~~~
87+
!!! error TS2694: Namespace '"tests/cases/conformance/salsa/mod".export=' has no exported member 'buz'.
88+
g: typeof import('./mod').literal,
89+
) {
90+
return a.length + b.length + c() + d() + e() + f() + g.length
91+
}
92+

‎tests/baselines/reference/moduleExportAssignment7.symbols

+195
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
=== tests/cases/conformance/salsa/mod.js ===
2+
class Thing { x = 1 }
3+
>Thing : Symbol(Thing, Decl(mod.js, 0, 0))
4+
>x : Symbol(Thing.x, Decl(mod.js, 0, 14))
5+
6+
class AnotherThing { y = 2 }
7+
>AnotherThing : Symbol(AnotherThing, Decl(mod.js, 0, 22))
8+
>y : Symbol(AnotherThing.y, Decl(mod.js, 1, 20))
9+
10+
function foo() { return 3 }
11+
>foo : Symbol(foo, Decl(mod.js, 1, 29))
12+
13+
function bar() { return 4 }
14+
>bar : Symbol(bar, Decl(mod.js, 2, 27))
15+
16+
/** @typedef {() => number} buz */
17+
module.exports = {
18+
>module.exports : Symbol(module.exports, Decl(mod.js, 0, 0))
19+
>module : Symbol(export=, Decl(mod.js, 3, 27))
20+
>exports : Symbol(export=, Decl(mod.js, 3, 27))
21+
22+
Thing,
23+
>Thing : Symbol(Thing, Decl(mod.js, 5, 18))
24+
25+
AnotherThing,
26+
>AnotherThing : Symbol(AnotherThing, Decl(mod.js, 6, 10))
27+
28+
foo,
29+
>foo : Symbol(foo, Decl(mod.js, 7, 17))
30+
31+
qux: bar,
32+
>qux : Symbol(qux, Decl(mod.js, 8, 8))
33+
>bar : Symbol(bar, Decl(mod.js, 2, 27))
34+
35+
baz() { return 5 },
36+
>baz : Symbol(baz, Decl(mod.js, 9, 13))
37+
38+
literal: "",
39+
>literal : Symbol(literal, Decl(mod.js, 10, 23))
40+
}
41+
=== tests/cases/conformance/salsa/main.js ===
42+
/**
43+
* @param {import("./mod").Thing} a
44+
* @param {import("./mod").AnotherThing} b
45+
* @param {import("./mod").foo} c
46+
* @param {import("./mod").qux} d
47+
* @param {import("./mod").baz} e
48+
* @param {import("./mod").buz} f
49+
* @param {import("./mod").literal} g
50+
*/
51+
function jstypes(a, b, c, d, e, f, g) {
52+
>jstypes : Symbol(jstypes, Decl(main.js, 0, 0))
53+
>a : Symbol(a, Decl(main.js, 9, 17))
54+
>b : Symbol(b, Decl(main.js, 9, 19))
55+
>c : Symbol(c, Decl(main.js, 9, 22))
56+
>d : Symbol(d, Decl(main.js, 9, 25))
57+
>e : Symbol(e, Decl(main.js, 9, 28))
58+
>f : Symbol(f, Decl(main.js, 9, 31))
59+
>g : Symbol(g, Decl(main.js, 9, 34))
60+
61+
return a.x + b.y + c() + d() + e() + f() + g.length
62+
>a.x : Symbol(Thing.x, Decl(mod.js, 0, 14))
63+
>a : Symbol(a, Decl(main.js, 9, 17))
64+
>x : Symbol(Thing.x, Decl(mod.js, 0, 14))
65+
>b.y : Symbol(AnotherThing.y, Decl(mod.js, 1, 20))
66+
>b : Symbol(b, Decl(main.js, 9, 19))
67+
>y : Symbol(AnotherThing.y, Decl(mod.js, 1, 20))
68+
>c : Symbol(c, Decl(main.js, 9, 22))
69+
>d : Symbol(d, Decl(main.js, 9, 25))
70+
>e : Symbol(e, Decl(main.js, 9, 28))
71+
>f : Symbol(f, Decl(main.js, 9, 31))
72+
>g.length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
73+
>g : Symbol(g, Decl(main.js, 9, 34))
74+
>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
75+
}
76+
77+
/**
78+
* @param {typeof import("./mod").Thing} a
79+
* @param {typeof import("./mod").AnotherThing} b
80+
* @param {typeof import("./mod").foo} c
81+
* @param {typeof import("./mod").qux} d
82+
* @param {typeof import("./mod").baz} e
83+
* @param {typeof import("./mod").buz} f
84+
* @param {typeof import("./mod").literal} g
85+
*/
86+
function jsvalues(a, b, c, d, e, f, g) {
87+
>jsvalues : Symbol(jsvalues, Decl(main.js, 11, 1))
88+
>a : Symbol(a, Decl(main.js, 22, 18))
89+
>b : Symbol(b, Decl(main.js, 22, 20))
90+
>c : Symbol(c, Decl(main.js, 22, 23))
91+
>d : Symbol(d, Decl(main.js, 22, 26))
92+
>e : Symbol(e, Decl(main.js, 22, 29))
93+
>f : Symbol(f, Decl(main.js, 22, 32))
94+
>g : Symbol(g, Decl(main.js, 22, 35))
95+
96+
return a.length + b.length + c() + d() + e() + f() + g.length
97+
>a.length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
98+
>a : Symbol(a, Decl(main.js, 22, 18))
99+
>length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
100+
>b.length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
101+
>b : Symbol(b, Decl(main.js, 22, 20))
102+
>length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
103+
>c : Symbol(c, Decl(main.js, 22, 23))
104+
>d : Symbol(d, Decl(main.js, 22, 26))
105+
>e : Symbol(e, Decl(main.js, 22, 29))
106+
>f : Symbol(f, Decl(main.js, 22, 32))
107+
>g.length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
108+
>g : Symbol(g, Decl(main.js, 22, 35))
109+
>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
110+
}
111+
112+
=== tests/cases/conformance/salsa/index.ts ===
113+
function types(
114+
>types : Symbol(types, Decl(index.ts, 0, 0))
115+
116+
a: import('./mod').Thing,
117+
>a : Symbol(a, Decl(index.ts, 0, 15))
118+
119+
b: import('./mod').AnotherThing,
120+
>b : Symbol(b, Decl(index.ts, 1, 29))
121+
122+
c: import('./mod').foo,
123+
>c : Symbol(c, Decl(index.ts, 2, 36))
124+
125+
d: import('./mod').qux,
126+
>d : Symbol(d, Decl(index.ts, 3, 27))
127+
128+
e: import('./mod').baz,
129+
>e : Symbol(e, Decl(index.ts, 4, 27))
130+
131+
f: import('./mod').buz,
132+
>f : Symbol(f, Decl(index.ts, 5, 27))
133+
>buz : Symbol(buz, Decl(mod.js, 4, 4))
134+
135+
g: import('./mod').literal,
136+
>g : Symbol(g, Decl(index.ts, 6, 27))
137+
138+
) {
139+
return a.x + b.y + c() + d() + e() + f() + g.length
140+
>a : Symbol(a, Decl(index.ts, 0, 15))
141+
>b : Symbol(b, Decl(index.ts, 1, 29))
142+
>c : Symbol(c, Decl(index.ts, 2, 36))
143+
>d : Symbol(d, Decl(index.ts, 3, 27))
144+
>e : Symbol(e, Decl(index.ts, 4, 27))
145+
>f : Symbol(f, Decl(index.ts, 5, 27))
146+
>g : Symbol(g, Decl(index.ts, 6, 27))
147+
}
148+
149+
function values(
150+
>values : Symbol(values, Decl(index.ts, 10, 1))
151+
152+
a: typeof import('./mod').Thing,
153+
>a : Symbol(a, Decl(index.ts, 12, 16))
154+
>Thing : Symbol(Thing, Decl(mod.js, 5, 18))
155+
156+
b: typeof import('./mod').AnotherThing,
157+
>b : Symbol(b, Decl(index.ts, 13, 36))
158+
>AnotherThing : Symbol(AnotherThing, Decl(mod.js, 6, 10))
159+
160+
c: typeof import('./mod').foo,
161+
>c : Symbol(c, Decl(index.ts, 14, 43))
162+
>foo : Symbol(foo, Decl(mod.js, 7, 17))
163+
164+
d: typeof import('./mod').qux,
165+
>d : Symbol(d, Decl(index.ts, 15, 34))
166+
>qux : Symbol(qux, Decl(mod.js, 8, 8))
167+
168+
e: typeof import('./mod').baz,
169+
>e : Symbol(e, Decl(index.ts, 16, 34))
170+
>baz : Symbol(baz, Decl(mod.js, 9, 13))
171+
172+
f: typeof import('./mod').buz,
173+
>f : Symbol(f, Decl(index.ts, 17, 34))
174+
175+
g: typeof import('./mod').literal,
176+
>g : Symbol(g, Decl(index.ts, 18, 34))
177+
>literal : Symbol(literal, Decl(mod.js, 10, 23))
178+
179+
) {
180+
return a.length + b.length + c() + d() + e() + f() + g.length
181+
>a.length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
182+
>a : Symbol(a, Decl(index.ts, 12, 16))
183+
>length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
184+
>b.length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
185+
>b : Symbol(b, Decl(index.ts, 13, 36))
186+
>length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
187+
>c : Symbol(c, Decl(index.ts, 14, 43))
188+
>d : Symbol(d, Decl(index.ts, 15, 34))
189+
>e : Symbol(e, Decl(index.ts, 16, 34))
190+
>f : Symbol(f, Decl(index.ts, 17, 34))
191+
>g.length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
192+
>g : Symbol(g, Decl(index.ts, 18, 34))
193+
>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
194+
}
195+

0 commit comments

Comments
 (0)