Style guide: Leading underscore conflict with TypeScript docs

[pburkindine]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[ ] Feature request
[x] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

The style guide states:

Avoid prefixing private properties and methods with an underscore.
https://angular.io/guide/styleguide#style-03-04

This is in conflict with how TypeScript documentation suggests we create getters and setters, and the behavior of both WebStorm and the prominent VSC extension when autogenerating getters and setters.

The pattern is like:

export class Foo {
  private _bar: string;

  ...

  public get bar(): string { return this._bar; }

Ref:

https://www.typescriptlang.org/docs/handbook/classes.html#accessors

johnpapa/angular-styleguide#861

Expected behavior

Style guide should either provide a recommendation on preferred alternative or remove this guideline.

Preferred alternative should be lintable, by e.g. tslint-consistent-codestyle.naming-convention (should be a universal rule for private fields and methods).

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?

Our organization is put in friction because we are required to encapsulate, required to follow TS documentation, and required to follow the style guide.

Environment

Angular version: N/A


Browser:
- [ ] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: XX  
- Platform:  

Others:

[ericmartinezr]

This has been discussed before #16893 (specially this one), angular/angular.io#1108, johnpapa/angular-styleguide#703. Really old issues though.

[pburkindine]

Yeah, have read the threads and I do understand the rationale. Unfortunately, it's in conflict with other official documentation and the behavior of the tooling. Docs should at minimum suggest an alternative convention imo.

[pburkindine]

Mentioned this to Google folks at Angular Mix this week and was advised to open an issue.

[nik72619c]

May I work on this if no one has taken it?

[manklu]

@pburkindine

You should notice that you have a special case. An attribut and a getter can't have the same name.

For the rest, get inspiration by the coding guidelines of the typescript project. https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines

Do not use "_" as a prefix for private properties.

[pburkindine]

I'm not sure what you mean. The case shown is the one from the TypeScript docs:

class Employee {
    private _fullName: string;

    get fullName(): string {
        return this._fullName;
}

If there is another pattern that is recommended, the style guide should include it. This pattern is the one autogenerated by the tooling in both Storm and VSC.

[manklu]

@pburkindine Yes, and the attribut must not have the same name as the getter.

The style guide says you should avoid it. If it is unavoidable, do what you want.

[pburkindine]

Still not sure what you mean... The attribute does not have the same name as the getter in the example shown. The leading underscore is the way the tooling and TS documentation are solving the issue that the names cannot be the same, as far as I can tell?

What I'm asking is that if this pattern is not to be used in Angular (this is avoid and not consider in the docs), an alternative pattern should be included. Our department guidelines and linting cannot abide the conflict in direction. We must encapsulate, and the way this is done in TypeScript by default is at odds with the Angular docs. Must enforce "the official answer" in the pipeline and can't at the moment.

[manklu]

@pburkindine You should avoid prefixing every private attribute with an underscrore. In your case it's the best solution to the problem.

IMO Avoid doesn't mean forbidden.

[pburkindine]

That answer would be fine if it could be included in the docs,

'Avoid using leading underscore, except for backing variables of public properties...'

From the linting perspective that's not an easy thing to enforce, though?

[pburkindine]

As far as avoid vs forbid goes, folks should understand that this style guide is being treated as "official best practices from Google" by consumers, and as such nobody's home if I go around claiming I think an exception should be made in such and such cases. If Google says avoid, we have to avoid, period - but here we can't, at least without an alternative pattern.

Since this is a very common use case, the style guide needs to address it imo.

[jenniferfell]

@IgorMinar : Please weigh in on this. Thanks!

[mikesm]

@pburkindine The TS handbook doesn't explicitly require prefixing with an underscore (even if in their example implementation they chose it for their workaround).

We do this by replacing the direct access to fullName with a set that will check the passcode. We add a corresponding get to allow the previous example to continue to work seamlessly.

If linting (or avoiding rather than considering) are issues then would using _ as a suffix work as a compromise for get/set? This seems to align with some existing guidelines in the ecosystem - e.g. $ suffixes for ngxs selectors - however personally I'm happy with prefixing.

IntelliJ allows editing of the templates used for autogenerating code, I'd expect that WebStorm supports template editing also.

[pburkindine]

@mikesm I am fine with the idea of a suffix. My main concern is that some official recommended workaround be included in the docs, even if it's vague like

<b>Consider</b> using a suffix on variables exposed by accessors/mutators

[julianpoemp]

Adding a new rule to my tslint.config fixed to me:

"variable-name": [
  true,
  "allow-leading-underscore"
]

I'm using PHPStorm and the "no-underscore" rule makes no sense to me. It's much easier and more efficient to use an leading underscore. Adding prefixes like "get" or "set" to getter/setters makes the code ugly. That's not Java, it's Typescript.

Furthermore the angular guidelines describes only that underscores shouldn't be used but explain nothing about any alternative when using getter/setters. Renaming the getter/setters manually is not an option.
https://angular.io/guide/styleguide#style-03-04

[pburkindine]

@jenniferfell @IgorMinar any update here?

[trotyl]

Closed by #30334, all these conventions have been removed.

[pburkindine]

@trotyl Thanks, I very much appreciate you all working on this

[cedvdb]

@pburkindine why did you close this ?

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}