Repository navigation
Declare property quick fix should add private properties (or be configurable) #36249
Description
Activity
a-tarasyuk commented
on Jan 28, 2020 ContributorMore actionsShould TS suggest a new QF for properties that start from
_(Declare private property '_bar')? OrDeclare property '_bar'QF should handle that?Reacted by Daniel RosenwasserDanielRosenwasser commented
on Jan 28, 2020 MemberMore actionsYeah, it sounds like the original quick fix is asking us to change the existing quick fix to add a
privatemodifier to the property in TypeScript files, and maybe a/** @private */doc comment in a JavaScript files?What's preferable here?
Reacted by Oleksandr Tarasiuk- addedDomain: LS: Quick FixesEditor-provided fixes, often called code actions.Editor-provided fixes, often called code actions.In DiscussionNot yet reached consensusNot yet reached consensusSuggestionAn idea for TypeScriptAn idea for TypeScript
on Jan 28, 2020 a-tarasyuk commented
on Jan 29, 2020 ContributorMore actionsDaniel Rosenwasser (@DanielRosenwasser)
TypeScript
class Foo { constructor() { this._bar = 1; /* Declare property 'bar' */ } }
After QF
class Foo { private _bar: number; constructor() { this._bar = 1; } }
Is it a right example for TypeScript?
The following example is a valid JavaScript code
class Foo { constructor() { this._bar = 1; } }
Where do we need to add
/** @private */JSDoc?Reacted by Daniel RosenwasserDanielRosenwasser commented
on Jan 29, 2020 MemberMore actionsYeah, I guess the JS case doesn't need that... hmm.
But yes, I think the TS example you had is what what the quick fix should do. It's not clear whether it should be a separate action or not though.
DanielRosenwasser commented
on Jan 29, 2020 MemberMore actionsI'm inclined to experiment with keeping it as part of the same action and seeing if users are unhappy about it.
DanielRosenwasser commented
on Jan 29, 2020 MemberMore actionsBut then I think there will be a question of "why don't you always make my properties private regardless of the
_?"Daniel Rosenwasser (@DanielRosenwasser) I personally don't tend to prefix privates with underscores, so I'd like it to be a separate action.
Of course if it's a separate action you'll soon have "why isn't there an action for protected if you allow private and public?".
a-tarasyuk commented
on Jan 31, 2020 ContributorMore actionsOf course if it's a separate action you'll soon have "why isn't there an action for protected if you allow private and public?".
I think, better to make a new action for that case. We can start by adding new QF action for
privatedeclaration, and see if it makes sense to add QF action forprotectedproperties. What do you think?cc Daniel Rosenwasser (@DanielRosenwasser) Jackson Kearl (@JacksonKearl)
Adding just a separate QF for private seems fine for now.
TypeScript Version: 3.8.0-beta
Search Terms:
Code
For the code:
Trigger the quick fix to declare a missing property on
_bar.Expected behavior:
The quick fix adds a private property
_bar:Actual behavior:
The quick fix adds a standard public property:
Playground Link:
Related Issues:
/cc Jackson Kearl (@JacksonKearl)