Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In JS, constructor functions without property assignments aren't recognised as classes #18171

Closed
mjbvz opened this issue Aug 30, 2017 · 6 comments · Fixed by #39447
Closed

In JS, constructor functions without property assignments aren't recognised as classes #18171

mjbvz opened this issue Aug 30, 2017 · 6 comments · Fixed by #39447
Assignees
Labels
Bug A bug in TypeScript Domain: JavaScript The issue relates to JavaScript specifically Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fix Available A PR has been opened for this issue VS Code Tracked There is a VS Code equivalent to this issue

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Aug 30, 2017

TypeScript Version: 2.5.1

Code

For the JS:

function Foo() {
	this.init();
}

Foo.prototype.init = function () {
	// code
}

const a = new Foo();
a.init()

Expected behavior:
In the function Foo, this is the type of Foo and you can run go to definition on this.init

Actual behavior:
this is any. go to definition does not work on this.init but does on a.init

@mjbvz mjbvz added the VS Code Tracked There is a VS Code equivalent to this issue label Aug 30, 2017
@mjbvz mjbvz changed the title JavaScript ES5 class constructor this type is 'any' JavaScript ES5 class constructor 'this' type is 'any' Aug 30, 2017
@mhegazy mhegazy added Bug A bug in TypeScript Salsa VS Code Tracked There is a VS Code equivalent to this issue and removed VS Code Tracked There is a VS Code equivalent to this issue labels Aug 31, 2017
@mhegazy mhegazy added this to the TypeScript 2.9 milestone Apr 12, 2018
@AlCalzone
Copy link
Contributor

I believe I'm running into a similar issue:

function Foo() {
	if (!(this instanceof Foo)) return new Foo();
	
	this.method1 = (abc) => {
		console.log(abc);
	};
	this.method1();

	this.method2 = () => {
		this.method1();
	};
}

method1 and method2 are correctly detected as properties of Foo:
unbenannt
However, inside the constructor and the methods, this is any and so is this.method1. Thus calling method1 with no parameters is not detected as an error.

@trusktr
Copy link
Contributor

trusktr commented May 17, 2019

Hello! How do we write ES5 constructor classes in TypeScript?

@sandersn
Copy link
Member

@trusktr This issue tracks detection of ES5 constructor functions in Javascript.

For example, the original example works if you put /** @class */ above it, but it's not detected otherwise because there's no this-property assignment, which is the other thing the compiler looks for.

To write an ES5 constructor function in Typescript, use this parameters. However, when you new this function, it'll still have type any, and an error in strict mode. You should use classes in Typescript if you want classes:

function Class(this: { x: number }, x: number) {
    this.x = x
}

var c = new Class(12) // this line is still an error in strict mode

@moll
Copy link

moll commented Sep 11, 2019

It's rather surprising the prototypical inheritance model with constructors isn't supported well in TypeScript. From https://github.com/microsoft/TypeScript/wiki/Type-Checking-JavaScript-Files#constructor-functions-are-equivalent-to-classes I got the impression the ES5 style is properly handled with new and this propagated as expected.

If the verbose solution of https://stackoverflow.com/questions/49799192/how-can-i-make-old-style-classes-work-in-typescript is truly by design, in what way are ES5 constructors functions equivalent as claimed in the wiki link)? It's nice that the methods get type checked, but a little useless if one can't instantiate the object with proper type inference. :P

Here's a test case for one parent and one inherited object: https://www.typescriptlang.org/play/index.html#code/GYVwdgxgLglg9mABABQKYCcDOCAUYCGAtqgFyKZToxgDmAlIgN4BQibiUAFjJgHQHFEAXkQDUzAL7NmoSLASIAqpgw4YAEzJgQhAEYYANKKKlylavSat2aLAl4R8AGyc4uPI2LrW27vhuFEDUlpZQxeAAd0OCgYgE8I1ECAeV0AK1RoB3RUfChUHFtsMEjo2KgE1CMWNggECnQQaDh0MkYAN2cQUzD0IzqwYBgaEHR8XSdTSm6jAHcqfPHJsmnUKQlvZk70RES7JBEwVFmUDGKcACIAKThOMAvvPeLeYDg4Zif7MWltxDTbg6iY5KFToHAAFgATEZrgCHsx-ndeMFESVvqiXm8gA

@moll
Copy link

moll commented Sep 11, 2019

@sandersn sandersn removed their assignment Jan 7, 2020
@DanielRosenwasser DanielRosenwasser added the Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. label Apr 29, 2020
@DanielRosenwasser DanielRosenwasser changed the title JavaScript ES5 class constructor 'this' type is 'any' Go-to-definition doesn't work on prototype properties in ES5 Apr 29, 2020
@DanielRosenwasser DanielRosenwasser added Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this labels Apr 29, 2020
@sandersn sandersn changed the title Go-to-definition doesn't work on prototype properties in ES5 In JS, constructor functions without property assignments aren't recognised as classes Apr 30, 2020
@sandersn
Copy link
Member

sandersn commented Jul 6, 2020

#39447 fixes the original example and the one from @AlCalzone, so I think it's fair to close this bug. However, the fix doesn't do exactly what the bug title requests. The fix types this: this inside any function that is known to be a constructor function, unlike 3.9, which only types this: this inside functions that have this-property assignments.

Almost every constructor function will have methods or a prototype assignment (or this-property assignments), so it covers basically all constructor functions.

@moll You are looking for #13206 or #36369
Please upvote and consider contributing some analysis of patterns you would like supported.

@sandersn sandersn added Fix Available A PR has been opened for this issue and removed Help Wanted You can do this labels Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: JavaScript The issue relates to JavaScript specifically Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fix Available A PR has been opened for this issue VS Code Tracked There is a VS Code equivalent to this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants