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

add special check for parameter initializer lookup if targeting es2015+ #26317

Merged

Conversation

Kingwl
Copy link
Contributor

@Kingwl Kingwl commented Aug 9, 2018

Fixes #22769

// to make sure that they reference no variables declared after them.
useResult =
// parameter initializer will lookup as normal variable scope when targeting es2015+
if (compilerOptions.target && compilerOptions.target >= ScriptTarget.ES2015 && isParameter(lastLocation) && lastLocation.initializer === originalLocation && result.valueDeclaration !== lastLocation) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't mention this in the original issue, but that's not limited to parameter intializers. It's basically every expression that could occur in a ParameterDeclaration. For example a ComputedPropertyName:

let foo = 0;

(function test({[foo]: bar}: any[]) {
    let foo = 1;
    console.log(bar); // logs 'a'
})(['a', 'b'])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is too desperate🤷‍♂️

@Kingwl Kingwl force-pushed the parameter-initializer-lookup-fix branch from c2351f5 to 8869f39 Compare August 28, 2018 08:52
@RyanCavanaugh RyanCavanaugh merged commit 6c2e851 into microsoft:master Aug 28, 2018
@weswigham
Copy link
Member

@RyanCavanaugh probably worth noting that even though this is a bugfix, it is also a breaking change. Also, in the context of parameter properties it doesn't make sense (ie, it is wrong), since those will still get downleveled, eg:

class Foo {
    constructor(public x = 12, public y = x.toFixed()) {}
}

we have breaks in RWC along these lines.

@sandersn
Copy link
Member

Filed #26719 to track the regression.

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.

5 participants