Conversation

RyanCavanaugh

Cleanup of #5578 that is based to the appropriate branch


// Look up the function in the local scope, since 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 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 assignment is immediately contained in a statement list and that the immediately preceding statement is a function declaration.

@ahejlsberg

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..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

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

@RyanCavanaugh

@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

👍

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

🎉

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

Successfully merging this pull request may close these issues.