Skip to content

Commit 4c68b6d

Browse files
rbucktonrbuckton
authored
Merge pull request microsoft#16686 from Microsoft/fix16467
Improve JavaScript type from constructor imported via require
2 parents f824e72 + 8ca66d3 commit 4c68b6d

4 files changed

Lines changed: 136 additions & 9 deletions

File tree

‎src/compiler/checker.ts‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16388,8 +16388,8 @@ namespace ts {
1638816388
* Indicates whether a declaration can be treated as a constructor in a JavaScript
1638916389
* file.
1639016390
*/
16391-
function isJavaScriptConstructor(node: Declaration): boolean {
16392-
if (isInJavaScriptFile(node)) {
16391+
function isJavaScriptConstructor(node: Declaration | undefined): boolean {
16392+
if (node && isInJavaScriptFile(node)) {
1639316393
// If the node has a @class tag, treat it like a constructor.
1639416394
if (getJSDocClassTag(node)) return true;
1639516395

@@ -16404,6 +16404,21 @@ namespace ts {
1640416404
return false;
1640516405
}
1640616406

16407+
function getJavaScriptClassType(symbol: Symbol): Type | undefined {
16408+
if (isDeclarationOfFunctionOrClassExpression(symbol)) {
16409+
symbol = getSymbolOfNode((<VariableDeclaration>symbol.valueDeclaration).initializer);
16410+
}
16411+
if (isJavaScriptConstructor(symbol.valueDeclaration)) {
16412+
return getInferredClassType(symbol);
16413+
}
16414+
if (symbol.flags & SymbolFlags.Variable) {
16415+
const valueType = getTypeOfSymbol(symbol);
16416+
if (valueType.symbol && !isInferredClassType(valueType) && isJavaScriptConstructor(valueType.symbol.valueDeclaration)) {
16417+
return getInferredClassType(valueType.symbol);
16418+
}
16419+
}
16420+
}
16421+
1640716422
function getInferredClassType(symbol: Symbol) {
1640816423
const links = getSymbolLinks(symbol);
1640916424
if (!links.inferredClassType) {
@@ -16447,16 +16462,14 @@ namespace ts {
1644716462
// in a JS file
1644816463
// Note:JS inferred classes might come from a variable declaration instead of a function declaration.
1644916464
// In this case, using getResolvedSymbol directly is required to avoid losing the members from the declaration.
16450-
let funcSymbol = node.expression.kind === SyntaxKind.Identifier ?
16465+
const funcSymbol = node.expression.kind === SyntaxKind.Identifier ?
1645116466
getResolvedSymbol(node.expression as Identifier) :
1645216467
checkExpression(node.expression).symbol;
16453-
if (funcSymbol && isDeclarationOfFunctionOrClassExpression(funcSymbol)) {
16454-
funcSymbol = getSymbolOfNode((<VariableDeclaration>funcSymbol.valueDeclaration).initializer);
16455-
}
16456-
if (funcSymbol && funcSymbol.flags & SymbolFlags.Function && (funcSymbol.members || getJSDocClassTag(funcSymbol.valueDeclaration))) {
16457-
return getInferredClassType(funcSymbol);
16468+
const type = funcSymbol && getJavaScriptClassType(funcSymbol);
16469+
if (type) {
16470+
return type;
1645816471
}
16459-
else if (noImplicitAny) {
16472+
if (noImplicitAny) {
1646016473
error(node, Diagnostics.new_expression_whose_target_lacks_a_construct_signature_implicitly_has_an_any_type);
1646116474
}
1646216475
return anyType;

‎tests/baselines/reference/constructorFunctions2.symbols‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
=== tests/cases/conformance/salsa/node.d.ts ===
2+
declare function require(id: string): any;
3+
>require : Symbol(require, Decl(node.d.ts, 0, 0))
4+
>id : Symbol(id, Decl(node.d.ts, 0, 25))
5+
6+
declare var module: any, exports: any;
7+
>module : Symbol(module, Decl(node.d.ts, 1, 11))
8+
>exports : Symbol(exports, Decl(node.d.ts, 1, 24))
9+
10+
=== tests/cases/conformance/salsa/index.js ===
11+
const A = require("./other");
12+
>A : Symbol(A, Decl(index.js, 0, 5))
13+
>require : Symbol(require, Decl(node.d.ts, 0, 0))
14+
>"./other" : Symbol("tests/cases/conformance/salsa/other", Decl(other.js, 0, 0))
15+
16+
const a = new A().id;
17+
>a : Symbol(a, Decl(index.js, 1, 5))
18+
>new A().id : Symbol(A.id, Decl(other.js, 0, 14))
19+
>A : Symbol(A, Decl(index.js, 0, 5))
20+
>id : Symbol(A.id, Decl(other.js, 0, 14))
21+
22+
const B = function() { this.id = 1; }
23+
>B : Symbol(B, Decl(index.js, 3, 5))
24+
>id : Symbol(B.id, Decl(index.js, 3, 22))
25+
26+
const b = new B().id;
27+
>b : Symbol(b, Decl(index.js, 4, 5))
28+
>new B().id : Symbol(B.id, Decl(index.js, 3, 22))
29+
>B : Symbol(B, Decl(index.js, 3, 5))
30+
>id : Symbol(B.id, Decl(index.js, 3, 22))
31+
32+
=== tests/cases/conformance/salsa/other.js ===
33+
function A() { this.id = 1; }
34+
>A : Symbol(A, Decl(other.js, 0, 0))
35+
>id : Symbol(A.id, Decl(other.js, 0, 14))
36+
37+
module.exports = A;
38+
>module : Symbol(export=, Decl(other.js, 0, 29))
39+
>exports : Symbol(export=, Decl(other.js, 0, 29))
40+
>A : Symbol(A, Decl(other.js, 0, 0))
41+

‎tests/baselines/reference/constructorFunctions2.types‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
=== tests/cases/conformance/salsa/node.d.ts ===
2+
declare function require(id: string): any;
3+
>require : (id: string) => any
4+
>id : string
5+
6+
declare var module: any, exports: any;
7+
>module : any
8+
>exports : any
9+
10+
=== tests/cases/conformance/salsa/index.js ===
11+
const A = require("./other");
12+
>A : () => void
13+
>require("./other") : () => void
14+
>require : (id: string) => any
15+
>"./other" : "./other"
16+
17+
const a = new A().id;
18+
>a : number
19+
>new A().id : number
20+
>new A() : { id: number; }
21+
>A : () => void
22+
>id : number
23+
24+
const B = function() { this.id = 1; }
25+
>B : () => void
26+
>function() { this.id = 1; } : () => void
27+
>this.id = 1 : 1
28+
>this.id : any
29+
>this : any
30+
>id : any
31+
>1 : 1
32+
33+
const b = new B().id;
34+
>b : number
35+
>new B().id : number
36+
>new B() : { id: number; }
37+
>B : () => void
38+
>id : number
39+
40+
=== tests/cases/conformance/salsa/other.js ===
41+
function A() { this.id = 1; }
42+
>A : () => void
43+
>this.id = 1 : 1
44+
>this.id : any
45+
>this : any
46+
>id : any
47+
>1 : 1
48+
49+
module.exports = A;
50+
>module.exports = A : () => void
51+
>module.exports : any
52+
>module : any
53+
>exports : any
54+
>A : () => void
55+

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// @allowJs: true
2+
// @checkJs: true
3+
// @noEmit: true
4+
// @module: commonjs
5+
// @filename: node.d.ts
6+
declare function require(id: string): any;
7+
declare var module: any, exports: any;
8+
9+
// @filename: index.js
10+
const A = require("./other");
11+
const a = new A().id;
12+
13+
const B = function() { this.id = 1; }
14+
const b = new B().id;
15+
16+
// @filename: other.js
17+
function A() { this.id = 1; }
18+
module.exports = A;

0 commit comments

Comments
 (0)