Skip to content

fix unnecessary Reference Check with property signature#23964

Closed
Kingwl wants to merge 2 commits intomicrosoft:masterfrom
Kingwl:unnecessary-property-signature
Closed

fix unnecessary Reference Check with property signature#23964
Kingwl wants to merge 2 commits intomicrosoft:masterfrom
Kingwl:unnecessary-property-signature

Conversation

@Kingwl
Copy link
Copy Markdown
Contributor

@Kingwl Kingwl commented May 8, 2018

Fixes #23829

Comment thread src/compiler/checker.ts
return true;
}

if (isComputedPropertyName(current) && isPropertySignature(current.parent) && current.parent.name === current) {
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.

that will also allow it inside the class body, which is not correct.

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.

class C{
    [C.a] = 0;
    static a = "a";
}

That would be throw based on the current TC39 proposal for class properties

Copy link
Copy Markdown
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

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

⬆️

@Kingwl
Copy link
Copy Markdown
Contributor Author

Kingwl commented May 24, 2018

@mhegazy could you give some example please?

@Kingwl Kingwl force-pushed the unnecessary-property-signature branch from 2a239d0 to c984361 Compare June 18, 2018 12:38
@RyanCavanaugh
Copy link
Copy Markdown
Member

Toggling to queue a fresh CI build

@RyanCavanaugh
Copy link
Copy Markdown
Member

Red CI and unaddressed comments for several months. Feel free to open a fresh PR as usual @Kingwl 👍

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

4 participants