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

JavaScript prototype class inference #5876

Merged
merged 15 commits into from
Dec 14, 2015

Conversation

RyanCavanaugh
Copy link
Member

Cleanup of #5578 that is based to the appropriate branch


// Look up the function in the local scope, since prototype assignments should immediately
// follow the function declaration
const funcSymbol = container.locals[classId.text];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't guarantee that the prototype assignment immediately follows. Also, what if classId denotes a non-function, such as a variable or an interface declaration? I'm thinking you should instead check that the prototype assignment is immediately contained in a statement list and that the immediately preceding statement is a function declaration.

@ahejlsberg
Copy link
Member

I'm thinking that the kind of inference you're doing should also apply when you access this in functions associated with the inferred "class". Specifically, within the constructor function and within methods (functions assigned to Foo.prototype.xxx), the type of this should be the inferred type. It seems odd to only infer the type outside the functions but not within them.

@RyanCavanaugh
Copy link
Member Author

Added this support for inferred method bodies, and added tests to verify that we don't require prototype assignments to do class inference

@RyanCavanaugh
Copy link
Member Author

@ahejlsberg any other comments?

// in a JS file
const funcSymbol = checkExpression(node.expression).symbol;
if (funcSymbol && funcSymbol.members && (funcSymbol.flags & SymbolFlags.Function)) {
return createAnonymousType(undefined, funcSymbol.members, emptyArray, emptyArray, /*stringIndexType*/ undefined, /*numberIndexType*/ undefined);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to create this type once and cache it. Right now you're creating a brand new type every time which allocates a lot of objects and defeats all the type comparison caching we do.

@ahejlsberg
Copy link
Member

👍

RyanCavanaugh added a commit that referenced this pull request Dec 14, 2015
@RyanCavanaugh RyanCavanaugh merged commit 2f447ee into microsoft:master Dec 14, 2015
@RyanCavanaugh RyanCavanaugh deleted the javaScriptPrototypes branch December 14, 2015 19:44
@DanielRosenwasser
Copy link
Member

🎉

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants