Skip to content

Fix inference for contextually typed parameters with initializers #29576

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

Merged
merged 8 commits into from
Jan 30, 2019

Conversation

ahejlsberg
Copy link
Member

Fixes #28816.


// Use the type of the initializer expression if one is present and the declaration is
// not a parameter of a contextually typed function
if (declaration.initializer && !isParameterOfContextuallyTypedFunction) {
Copy link
Member

@weswigham weswigham Jan 25, 2019

Choose a reason for hiding this comment

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

Wouldn't we rather write declaration.initializer && !(declaration.kind === SyntaxKind.Parameter && getContextualType(<Expression>declaration.parent)) so we only call getContextualType if we absolutely need to?

Copy link
Member

Choose a reason for hiding this comment

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

Or, since it's used below, write

const needsShapeAndIsNotParameterOfContextuallyTypedFunction = (declaration.initializer || isBindingPattern(declaration.name)) && !(declaration.kind === SyntaxKind.Parameter && getContextualType(<Expression>declaration.parent));

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, possibly. The value is precomputed because I use it in two locations. But I supposed I could turn it into a function that we only call when appropriate.

declare function id5<T extends (x?: number) => any>(input: T): T;

const f10 = function ({ foo = 42 }) { return foo };
const f11 = id1(function ({ foo = 42 }) { return foo }); // Implicit any error
Copy link
Member

Choose a reason for hiding this comment

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

Should we also have a fourslash test for each of these so we can have tests simulating hovering over the different parts in different orders?

const f10 = function ({ foo = 42 }) { return foo };
const f11 = id1(function ({ foo = 42 }) { return foo }); // Implicit any error
~~~~~~~~~~~~
!!! error TS7022: '{ foo = 42 }' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.
Copy link
Member

Choose a reason for hiding this comment

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

How does this binding pattern circularly reference itself? o.O

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, strange. This error is caused by declaration file emit, it only appears with --declaration. Have no idea why.

Copy link
Member

Choose a reason for hiding this comment

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

getEmitDeclarations causes us to conditionally collectLinkedAliases on exported names - but this is a binding pattern in an anonymous function expression, not an exported name... o.O

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we generate the error while looking up the type of the expression for emitting declare const f11: something? We marked foo as implicitly any within the checker, however did we propagate that up to the binding pattern as a whole?

Copy link
Member Author

Choose a reason for hiding this comment

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

The circularity error is caused by the parameter having itself as its contextual type. It's actually unrelated to this PR. For example, this causes the same error on master:

declare function id<T>(input: T): T;
const f1 = id(function ({ foo }) { return foo });

# Conflicts:
#	src/compiler/checker.ts
@ahejlsberg ahejlsberg merged commit eb513a2 into master Jan 30, 2019
@ahejlsberg ahejlsberg deleted the fixContextuallyTypedParameters branch January 30, 2019 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants