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

Option for "variable-name" rule to allow leading underscore for property backing fields #1489

Closed
glen-84 opened this issue Aug 17, 2016 · 7 comments

Comments

@glen-84
Copy link
Contributor

@glen-84 glen-84 commented Aug 17, 2016

It would be nice to have this:

class Example {
    private _var1; // ERROR, not a backing field, shouldn't start with an underscore.
    private _var2; // Allowed, as it's a backing field for the `var2` property.

    public get var2() {
        return this._var2;
    }
}

allow-leading-underscore-backing-fields maybe.

Edit: It would be considered a backing field if there was either a getter or a setter with the same name (sans the leading underscore).

@stevedenman

This comment has been minimized.

Copy link

@stevedenman stevedenman commented Sep 20, 2016

This naming convention for backing fields is also suggested in the typescript handbook ;

private _fullName: string;

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

This comment has been minimized.

Copy link
Collaborator

@JoshuaKGoldberg JoshuaKGoldberg commented Nov 3, 2018

variable-name now allows options such as "allow-leading-underscore". 🎉

@glen-84

This comment has been minimized.

Copy link
Contributor Author

@glen-84 glen-84 commented Nov 3, 2018

@JoshuaKGoldberg That's not new, and allows leading underscores on any variable.

@JoshuaKGoldberg

This comment has been minimized.

Copy link
Collaborator

@JoshuaKGoldberg JoshuaKGoldberg commented Nov 3, 2018

Oh! My apologies, I speed-read the rule and missed the difference. Agreed that an option to allow a leading underscore only for backing properties would be nice.

We should note that it shouldn't be allowed to be enabled at the same time as "allow-leading-underscore".

emizzle added a commit to embarklabs/embark that referenced this issue Dec 3, 2018
The webpack process took quite a while to run, and there were no updates in the console while running.

This PR adds a spinner (when there is no dashboard) and status updates every 5 seconds. When there is a dashboard, the updates are added to a new line.

After (with dashboard):
![with dashboard](https://i.imgur.com/zVJH5U4.png)

After (`—nodashboard`):
![no dashboard](http://g.recordit.co/2zRNLt51jU.gif)

Convert LongRunningProcessTimer to TypeScript

PR feedback and consistency changes

Changed the constructor signature to accept an options object instead of individual optional parameters, for readability.

Changed library_manager to use the spinner when not using the dashboard, for consistency’s sake. Additionally increased the update time for the library manager from 750ms to 1s.

Fix lint errors

Added `"variable-name": ["allow-leading-underscore”]` to `tslint.json` due to a lack of the ability to prefix backing variables with underscore. This is an [ongoing discussion](palantir/tslint#1489), and something the community thinks should be implemented, as it the preferred way to use a property with backing variable in TypeScript.
emizzle added a commit to embarklabs/embark that referenced this issue Dec 4, 2018
The webpack process took quite a while to run, and there were no updates in the console while running.

This PR adds a spinner (when there is no dashboard) and status updates every 5 seconds. When there is a dashboard, the updates are added to a new line.

After (with dashboard):
![with dashboard](https://i.imgur.com/zVJH5U4.png)

After (`—nodashboard`):
![no dashboard](http://g.recordit.co/2zRNLt51jU.gif)

Convert LongRunningProcessTimer to TypeScript

PR feedback and consistency changes

Changed the constructor signature to accept an options object instead of individual optional parameters, for readability.

Changed library_manager to use the spinner when not using the dashboard, for consistency’s sake. Additionally increased the update time for the library manager from 750ms to 1s.

Fix lint errors

Added `"variable-name": ["allow-leading-underscore”]` to `tslint.json` due to a lack of the ability to prefix backing variables with underscore. This is an [ongoing discussion](palantir/tslint#1489), and something the community thinks should be implemented, as it the preferred way to use a property with backing variable in TypeScript.
iurimatias added a commit to embarklabs/embark that referenced this issue Dec 7, 2018
The webpack process took quite a while to run, and there were no updates in the console while running.

This PR adds a spinner (when there is no dashboard) and status updates every 5 seconds. When there is a dashboard, the updates are added to a new line.

After (with dashboard):
![with dashboard](https://i.imgur.com/zVJH5U4.png)

After (`—nodashboard`):
![no dashboard](http://g.recordit.co/2zRNLt51jU.gif)

Convert LongRunningProcessTimer to TypeScript

PR feedback and consistency changes

Changed the constructor signature to accept an options object instead of individual optional parameters, for readability.

Changed library_manager to use the spinner when not using the dashboard, for consistency’s sake. Additionally increased the update time for the library manager from 750ms to 1s.

Fix lint errors

Added `"variable-name": ["allow-leading-underscore”]` to `tslint.json` due to a lack of the ability to prefix backing variables with underscore. This is an [ongoing discussion](palantir/tslint#1489), and something the community thinks should be implemented, as it the preferred way to use a property with backing variable in TypeScript.
@Xenya0815

This comment has been minimized.

Copy link

@Xenya0815 Xenya0815 commented Feb 5, 2019

The issue is closed but I can't find a option at the documentation. Is it still planned to offer a option like allow-leading-underscore-backing-fields?

@JoshuaKGoldberg

This comment has been minimized.

Copy link
Collaborator

@JoshuaKGoldberg JoshuaKGoldberg commented Feb 5, 2019

Thanks for the catch @Xenya0815 - this should have been re-opened.

@adidahiya

This comment has been minimized.

Copy link
Member

@adidahiya adidahiya commented Sep 10, 2019

Closing in favor of typescript-eslint/typescript-eslint#816

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.