Skip to content

fix(48191): Duplicates comments on "Add definite assignment assertion to property" #48299

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

Conversation

MQuy
Copy link
Contributor

@MQuy MQuy commented Mar 16, 2022

Fixes #48191

When fixing #48191, I found the similar issue happen for Add initializer to property which is fixed in this.

Reproduce link: https://www.typescriptlang.org/play?#code/MYGwhgzhAECC0G8BQ1oHo3QBYFMQgHsVoAPALmgCYkBfJIA

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Mar 16, 2022
@ghost
Copy link

ghost commented Mar 16, 2022

CLA assistant check
All CLA requirements met.

@MQuy MQuy changed the title Supress comments for class property initialization fix(48191): Duplicates comments on "Add definite assignment assertion to property" Mar 16, 2022
@DanielRosenwasser
Copy link
Member

I found the similar issue happen for Generate "get" and "set" accessor which is fixed in this.

Do you mean the quick fix for adding undefined?

@MQuy
Copy link
Contributor Author

MQuy commented Mar 16, 2022

thank @DanielRosenwasser for your reply. Sorry, I wrote the incorrect description and tests which are fixed now, it includes a fix for the case

class A {
  // hello
  x: 2
}

->

class A {
  // hello
  x: 2 = 2;
}

@andrewbranch
Copy link
Member

I think there’s a way to accomplish the same thing by using the options parameter of replaceNode to control what leading and trailing trivia gets replaced—i.e., instead of suppressing trivia emit and replacing only the non-trivia range of the node, you could replace the existing trivia and let it re-print from the emitter. However, making the smallest possible text edits is a good operating principle for code fixes, so I’m happy with your fix.

@andrewbranch andrewbranch merged commit b996287 into microsoft:main Mar 17, 2022
@MQuy MQuy deleted the supress-comments-for-class-property-initialization branch March 17, 2022 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quick-fix duplicates comments on "Add definite assignment assertion to property"
4 participants