Skip to content

Contextually type binding initializers#7298

Merged
sandersn merged 5 commits into
masterfrom
contextually-type-binding-initializers
Feb 29, 2016
Merged

Contextually type binding initializers#7298
sandersn merged 5 commits into
masterfrom
contextually-type-binding-initializers

Conversation

@sandersn
Copy link
Copy Markdown
Member

Fixes #5762

Previously they were not contextually typed, which meant that lambdas got completely incorrect types, and that types that rely on contextual typing, like tuples and string literal types, did not work correctly.

Previously they were not contextually typed, which meant that lambdas got
completely incorrect types, and that types that rely on contextual typing,
like tuples and string literal types, did not work correctly.
@sandersn
Copy link
Copy Markdown
Member Author

I forgot to re-run tests after merging from master and one of the new tests failed. I'll fix it and update the PR.

@sandersn
Copy link
Copy Markdown
Member Author

The baselines were missing Mohamed's improved serialisation of binding elements that doesn't print the initialiser as part of the type.

Comment thread src/compiler/checker.ts Outdated
const name = declaration.propertyName || declaration.name;
if (isVariableLike(parentDeclaration) &&
parentDeclaration.type &&
(name.kind === SyntaxKind.Identifier || name.kind == SyntaxKind.StringLiteral)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You don't have any tests right now for when the name is a string literal.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In which case, wouldn't those properties be computed? What about the following cases?

This should have no errors:

interface Foo {
    prop: (x: string) => number;
}

function f({ ["prop"]: local = v => +v }: Foo) {
}

This should have an error:

interface Foo {
    prop: (x: string) => number;
}

function g({ ["prop"]: local = v => v }: Foo) {
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This JSON-like syntax is allowed, which is the StringLiteral case, I believe:

function f( { "prop": local = v => +v }: Foo) {
}

The code you give is a ComputedPropertyName, I believe. I'll test all four.

Also, I used == instead of === by mistake. I'm not sure how the linter missed that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yup, looks like ComputedPropertyName was missing. Thanks for the help finding and fixing the oversight.

With tests and associated baseline updates
@DanielRosenwasser
Copy link
Copy Markdown
Member

👍

sandersn added a commit that referenced this pull request Feb 29, 2016
…tializers

Contextually type binding initializers
@sandersn sandersn merged commit 8a72229 into master Feb 29, 2016
@sandersn sandersn deleted the contextually-type-binding-initializers branch February 29, 2016 22:26
@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.

3 participants