-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Add 'prefixText' and 'suffixText' when renaming shorthand properties #27356
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
d8fb299
to
232e353
Compare
return unwrapJSONCallResult(this.shim.findRenameLocations(fileName, position, findInStrings, findInComments)); | ||
const res = unwrapJSONCallResult(this.shim.findRenameLocations(fileName, position, findInStrings, findInComments)) as ReadonlyArray<ts.RenameLocation>; | ||
// JSON serialization removes undefined properties, so add them back if necessary. | ||
return res.map((r): ts.RenameLocation => ({ prefixText: undefined, suffixText: undefined, ...r })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? Not having those properties should result in same result as having them as undefined?
src/server/protocol.ts
Outdated
} | ||
|
||
export interface RenameTextSpan extends TextSpan { | ||
readonly prefixText: string | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make these optional instead?
ef92a72
to
b7e3dbf
Compare
b7e3dbf
to
6a8f472
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Protocol changes look good to me. Once this is merged, I'll add basic support in VS Code so users of TS@next can try it out before we pick up TS 3.2
@sheetalkamat Good to merge? |
What if I want to call tsLanguageService.findRenameLocations(...) to rename the key of short hand property? It seems that I cannot use this api to achieve my goal right now. |
Fixes #12007