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

[1.30.0] Rename ES6 object-property inside method-parameter not working #29029

Closed
Friksel opened this issue Dec 13, 2018 · 9 comments
Closed
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@Friksel
Copy link

Friksel commented Dec 13, 2018

After updating Vs Code to 1.30.0 renaming objects-properties used as method-parameters in an ES6 class don't rename as they should.

rename-not-working

  • VSCode Version: 1.30.0
  • OS Version: Windows 10

Steps to Reproduce:

  1. Have a class with an object as param
  2. Rename one property of that object

Does this issue occur when all extensions are disabled?: Yes

@Friksel Friksel changed the title [1.30.0] Rename ES6 class-variable inside object not working [1.30.0] Rename ES6 object-property inside method-parameter not working Dec 13, 2018
@mjbvz mjbvz self-assigned this Dec 13, 2018
@mjbvz mjbvz transferred this issue from microsoft/vscode Dec 14, 2018
@mjbvz mjbvz removed their assignment Dec 14, 2018
@weswigham
Copy link
Member

I don't think this is a bug - we rename the name used within the class, but not the name used outside of it, since that's a separate symbol (though a destructuring operation ties them together).

@weswigham
Copy link
Member

Like, this is so intentional that it's listed as a feature on the release announcement: https://code.visualstudio.com/updates/v1_30#_renames-handle-jsts-destructuring-properly

@DanielRosenwasser DanielRosenwasser added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Dec 14, 2018
@Friksel
Copy link
Author

Friksel commented Dec 15, 2018

Yes this is a bug. It's unwanted behaviour and it's doing something completely different then renaming. Look at the video in my post. This is not renaming, this is adding something and changing a structure into something we don't want in Javascript and doesn't work.

The link you sent is clearly a different situation; that's about interfaces. Next to that renaming looks to work on that example, but isn't in this situation in 1.30.0.

This is how it was working in 1.29.1 and is suppose to work:
renaming-in-1 29 1

@Friksel
Copy link
Author

Friksel commented Dec 27, 2018

Still happening in 1.30.1. Any news on this?

@justingrant
Copy link
Contributor

Dupe of #29238?

@weswigham
Copy link
Member

A per-user setting is being implemented at #29593

@justingrant
Copy link
Contributor

justingrant commented Feb 11, 2019

@Friksel - Looks like this PR #29593 will be included in TypeScript 3.4 which should be out in a month or two.

@Friksel
Copy link
Author

Friksel commented Feb 12, 2019

Thanks for the updates guys @justingrant @weswigham . Great it's changing!

@Friksel
Copy link
Author

Friksel commented Jul 20, 2019

@justingrant I see the PR you are talking about is merged to master a while ago and this issue is closed, but in version 1.36.1 the result I get is still exactly the same as when I opened this issue, so so far I don't see any change.

Do I perhaps need to change some setting to make this work as expected?

[edit] Ah, my bad. I just in the commit message of the PR a setting has indeed been added. For future reference for people having the same issue:

  • Via Settings UI: The setting is called Javascript --> Preferences --> Rename Shorthand Properties in the UI and if you remove the tick it's working as expected in Javascript.
  • Via settings.json: Or just add "javascript.preferences.renameShorthandProperties": false to the settings.json

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants