Skip to content

Commit 8a4b6e0

Browse files
committed
Fix class/constructor-function merge (microsoft#27366)
The check for prototype assignment on constructor functions assumes that the prototype property, if present, comes from an assignment declaration, such as: ```js SomeClass.prototype = { /* methods go here */ } ``` In this case, however, when class SomeClass and var SomeClass merge (because this is allowed), prototype is the synthetic property from class SomeClass, which has no valueDeclaration. The fix is to check that prototype has a valueDeclaration before checking whether the valueDeclaration is in fact a prototype-assignment declaration.
1 parent a6a27e9 commit 8a4b6e0

File tree

4 files changed

+65
-1
lines changed

4 files changed

+65
-1
lines changed

src/compiler/checker.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20459,7 +20459,7 @@ namespace ts {
2045920459
isBinaryExpression(decl.parent) && getSymbolOfNode(decl.parent.left) ||
2046020460
isVariableDeclaration(decl.parent) && getSymbolOfNode(decl.parent));
2046120461
const prototype = assignmentSymbol && assignmentSymbol.exports && assignmentSymbol.exports.get("prototype" as __String);
20462-
const init = prototype && getAssignedJSPrototype(prototype.valueDeclaration);
20462+
const init = prototype && prototype.valueDeclaration && getAssignedJSPrototype(prototype.valueDeclaration);
2046320463
return init ? checkExpression(init) : undefined;
2046420464
}
2046520465

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
=== tests/cases/conformance/salsa/file1.js ===
2+
var SomeClass = function () {
3+
>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19))
4+
5+
this.otherProp = 0;
6+
>otherProp : Symbol(SomeClass.otherProp, Decl(file1.js, 0, 29))
7+
8+
};
9+
10+
new SomeClass();
11+
>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19))
12+
13+
=== tests/cases/conformance/salsa/file2.js ===
14+
class SomeClass { }
15+
>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19))
16+
17+
SomeClass.prop = 0
18+
>SomeClass.prop : Symbol(SomeClass.prop, Decl(file2.js, 0, 19))
19+
>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19))
20+
>prop : Symbol(SomeClass.prop, Decl(file2.js, 0, 19))
21+
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
=== tests/cases/conformance/salsa/file1.js ===
2+
var SomeClass = function () {
3+
>SomeClass : typeof SomeClass
4+
>function () { this.otherProp = 0;} : typeof SomeClass
5+
6+
this.otherProp = 0;
7+
>this.otherProp = 0 : 0
8+
>this.otherProp : any
9+
>this : any
10+
>otherProp : any
11+
>0 : 0
12+
13+
};
14+
15+
new SomeClass();
16+
>new SomeClass() : SomeClass
17+
>SomeClass : typeof SomeClass
18+
19+
=== tests/cases/conformance/salsa/file2.js ===
20+
class SomeClass { }
21+
>SomeClass : SomeClass
22+
23+
SomeClass.prop = 0
24+
>SomeClass.prop = 0 : 0
25+
>SomeClass.prop : number
26+
>SomeClass : typeof SomeClass
27+
>prop : number
28+
>0 : 0
29+
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// @allowJs: true
2+
// @noEmit: true
3+
// @checkJs: true
4+
5+
// @Filename: file1.js
6+
var SomeClass = function () {
7+
this.otherProp = 0;
8+
};
9+
10+
new SomeClass();
11+
12+
// @Filename: file2.js
13+
class SomeClass { }
14+
SomeClass.prop = 0

0 commit comments

Comments
 (0)