GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Declare property quick fix should add private properties (or be configurable) · Issue #36249 · microsoft/TypeScript

Declare property quick fix should add private properties (or be configurable)  #36249

New issue
Copy link
New issue
Copy link
Closed
#36632
Closed
Declare property quick fix should add private properties (or be configurable) #36249
#36632
Copy link
Labels
Domain: 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
Milestone
TypeScript 3.9.0

Description

@mjbvz
Matt Bierner (mjbvz)
opened on Jan 17, 2020
Issue body actions

TypeScript Version: 3.8.0-beta

Search Terms:

  • quick fix
  • declare property

Code
For the code:

class Foo { 
	constructor() { 
		this._bar = 1;
	}
}

new Foo().other = 'a';

Trigger the quick fix to declare a missing property on _bar.

Expected behavior:
The quick fix adds a private property _bar:

class Foo { 
	private _bar: number;     
	constructor() { 
		this._bar = 1;
	}
}

Actual behavior:
The quick fix adds a standard public property:

class Foo {
	_bar: number; 
	constructor() { 
		this._bar = 1;
	}
}

Playground Link:

Related Issues:

/cc Jackson Kearl (@JacksonKearl)

Activity

  1. a-tarasyuk commented on Jan 28, 2020

    @a-tarasyuk
    Oleksandr Tarasiuk (a-tarasyuk)
    on Jan 28, 2020
    Contributor
    More actions

    Should TS suggest a new QF for properties that start from _ (Declare private property '_bar')? Or Declare property '_bar' QF should handle that?

    cc Daniel Rosenwasser (@DanielRosenwasser)

    Reacted by Daniel Rosenwasser
  2. DanielRosenwasser commented on Jan 28, 2020

    @DanielRosenwasser
    Daniel Rosenwasser (DanielRosenwasser)
    on Jan 28, 2020
    Member
    More actions

    Yeah, it sounds like the original quick fix is asking us to change the existing quick fix to add a private modifier to the property in TypeScript files, and maybe a /** @private */ doc comment in a JavaScript files?

    What's preferable here?

    Reacted by Oleksandr Tarasiuk
  3. Daniel Rosenwasser (DanielRosenwasser)
    added
    Domain: LS: Quick FixesEditor-provided fixes, often called code actions.Editor-provided fixes, often called code actions.
    In DiscussionNot yet reached consensusNot yet reached consensus
    SuggestionAn idea for TypeScriptAn idea for TypeScript
    on Jan 28, 2020
  4. a-tarasyuk commented on Jan 29, 2020

    @a-tarasyuk
    Oleksandr Tarasiuk (a-tarasyuk)
    on Jan 29, 2020
    Contributor
    More actions

    Daniel 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 Rosenwasser
  5. DanielRosenwasser commented on Jan 29, 2020

    @DanielRosenwasser
    Daniel Rosenwasser (DanielRosenwasser)
    on Jan 29, 2020
    Member
    More actions

    Yeah, 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.

  6. DanielRosenwasser commented on Jan 29, 2020

    @DanielRosenwasser
    Daniel Rosenwasser (DanielRosenwasser)
    on Jan 29, 2020
    Member
    More actions

    I'm inclined to experiment with keeping it as part of the same action and seeing if users are unhappy about it.

  7. DanielRosenwasser commented on Jan 29, 2020

    @DanielRosenwasser
    Daniel Rosenwasser (DanielRosenwasser)
    on Jan 29, 2020
    Member
    More actions

    But then I think there will be a question of "why don't you always make my properties private regardless of the _?"

  8. JacksonKearl commented on Jan 29, 2020

    @JacksonKearl
    Jackson Kearl (JacksonKearl)
    on Jan 29, 2020
    More actions

    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?".

  9. a-tarasyuk commented on Jan 31, 2020

    @a-tarasyuk
    Oleksandr Tarasiuk (a-tarasyuk)
    on Jan 31, 2020
    Contributor
    More actions

    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?".

    I think, better to make a new action for that case. We can start by adding new QF action for private declaration, and see if it makes sense to add QF action for protected properties. What do you think?

    cc Daniel Rosenwasser (@DanielRosenwasser) Jackson Kearl (@JacksonKearl)

  10. JacksonKearl commented on Jan 31, 2020

    @JacksonKearl
    Jackson Kearl (JacksonKearl)
    on Jan 31, 2020
    More actions

    Adding just a separate QF for private seems fine for now.

  11. Andrew Casey (amcasey)
    closed this as completedin #36632on Mar 13, 2020
  12. Daniel Rosenwasser (DanielRosenwasser)
    modified the milestones: TypeScript 3.9.1, TypeScript 3.9.0 on Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Domain: 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

    Type

    No type

    Projects

    No projects

      Milestone

      • TypeScript 3.9.0

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions

      You are visitor 01402503 — GeezerVison does not store the pages you look up.
      Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.