Skip to content

Commit

Permalink
Fix class/constructor-function merge (#27366)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sandersn committed Oct 8, 2018
1 parent 1cfab76 commit 1d773a1
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20331,7 +20331,7 @@ namespace ts {
isBinaryExpression(decl.parent) && getSymbolOfNode(decl.parent.left) ||
isVariableDeclaration(decl.parent) && getSymbolOfNode(decl.parent));
const prototype = assignmentSymbol && assignmentSymbol.exports && assignmentSymbol.exports.get("prototype" as __String);
const init = prototype && getAssignedJSPrototype(prototype.valueDeclaration);
const init = prototype && prototype.valueDeclaration && getAssignedJSPrototype(prototype.valueDeclaration);
return init ? checkExpression(init) : undefined;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
=== tests/cases/conformance/salsa/file1.js ===
var SomeClass = function () {
>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19))

this.otherProp = 0;
>otherProp : Symbol(SomeClass.otherProp, Decl(file1.js, 0, 29))

};

new SomeClass();
>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19))

=== tests/cases/conformance/salsa/file2.js ===
class SomeClass { }
>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19))

SomeClass.prop = 0
>SomeClass.prop : Symbol(SomeClass.prop, Decl(file2.js, 0, 19))
>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19))
>prop : Symbol(SomeClass.prop, Decl(file2.js, 0, 19))

29 changes: 29 additions & 0 deletions tests/baselines/reference/constructorFunctionMergeWithClass.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
=== tests/cases/conformance/salsa/file1.js ===
var SomeClass = function () {
>SomeClass : typeof SomeClass
>function () { this.otherProp = 0;} : typeof SomeClass

this.otherProp = 0;
>this.otherProp = 0 : 0
>this.otherProp : any
>this : any
>otherProp : any
>0 : 0

};

new SomeClass();
>new SomeClass() : SomeClass
>SomeClass : typeof SomeClass

=== tests/cases/conformance/salsa/file2.js ===
class SomeClass { }
>SomeClass : SomeClass

SomeClass.prop = 0
>SomeClass.prop = 0 : 0
>SomeClass.prop : number
>SomeClass : typeof SomeClass
>prop : number
>0 : 0

14 changes: 14 additions & 0 deletions tests/cases/conformance/salsa/constructorFunctionMergeWithClass.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// @allowJs: true
// @noEmit: true
// @checkJs: true

// @Filename: file1.js
var SomeClass = function () {
this.otherProp = 0;
};

new SomeClass();

// @Filename: file2.js
class SomeClass { }
SomeClass.prop = 0

0 comments on commit 1d773a1

Please sign in to comment.