Skip to content

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

Merged
Ryan Cavanaugh (RyanCavanaugh) merged 2 commits into
microsoft:masterfrom
Kingwl:parameter-initializer-lookup-fix
Aug 28, 2018
Merged

Ryan Cavanaugh (RyanCavanaugh) merged 2 commits into
microsoft:masterfrom
Kingwl:parameter-initializer-lookup-fix

Conversation

@Kingwl

Copy link
Copy Markdown
Contributor

Fixes #22769

Comment thread src/compiler/checker.ts Outdated
// 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
Copy Markdown
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
Copy Markdown
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
Wenlu Wang (Kingwl) force-pushed the parameter-initializer-lookup-fix branch from c2351f5 to 8869f39 Compare August 28, 2018 08:52
@RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) merged commit 6c2e851 into microsoft:master Aug 28, 2018
@weswigham

Copy link
Copy Markdown
Member

Ryan Cavanaugh (@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
Copy Markdown
Member

Filed #26719 to track the regression.

Ryan Cavanaugh (RyanCavanaugh) added a commit to RyanCavanaugh/TypeScript that referenced this pull request Aug 31, 2018
…ration for a clean PR fixing all the issues.
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
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.

5 participants