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

Rule suggestion: no-var (node) #633

Open
albinekb opened this issue Sep 19, 2016 · 17 comments

Comments

@albinekb
Copy link

commented Sep 19, 2016

Referring to this rule.

Standard should read node version from the engines (more information) property in package.json, if the version supports const, no-var should be added to the list of rules.

Any thoughts?
Is there any reason to use var in node versions that support const and let?

@dcousens

This comment has been minimized.

Copy link
Member

commented Sep 19, 2016

Would probably break wayyy to much. Although, I agree with the rule fundamentally.
Might be worth linking in #523

@albinekb

This comment has been minimized.

Copy link
Author

commented Sep 19, 2016

Maybe....

Will standard never release a breaking version, where developers need to fix their code? 😏

I mean, there must be room to add rules that break some codebases to make standard stricter (and thus better)?

@dcousens

This comment has been minimized.

Copy link
Member

commented Sep 19, 2016

#382 might be what you're after, but in the end, I write code for environments where let isn't defined, so this would stop me from using standard since my local node version is >6.0.0.

@albinekb

This comment has been minimized.

Copy link
Author

commented Sep 19, 2016

Cool!
Yes but if your code supports environments where let isn't defined, you don't have engines in your package.json set to node 6? 💭

See https://docs.npmjs.com/files/package.json#engines

@dcousens

This comment has been minimized.

Copy link
Member

commented Sep 20, 2016

Isn't engines only enforced if the user specifies engines-strict? Which is default off...

@feross

This comment has been minimized.

Copy link
Member

commented Sep 20, 2016

@dcousens I believe that npm will still print a warning in that case.

Anyway, I don't think that magical behavior like changing rules based on engines field is the way to go.

Once let works in more environments (like 90% of browsers on CanIUse I think we can consider enabling it. There's definitely an argument to be made that var serves no purpose once let is pervasive. Also, this rule is automatically fixable.

@dcousens

This comment has been minimized.

Copy link
Member

commented Sep 20, 2016

Agreed @feross, I'm just saying I don't think that time is now. Hopefully soon though :)

@feross

This comment has been minimized.

Copy link
Member

commented Sep 26, 2016

Cool -- I'll close this issue for now since it's not actionable.

@feross feross closed this Sep 26, 2016

@ForsakenHarmony

This comment has been minimized.

Copy link

commented Oct 13, 2017

currently at 83%

@redbmk

This comment has been minimized.

Copy link

commented Apr 4, 2018

Up to 91.4% now

@LinusU

This comment has been minimized.

Copy link
Member

commented Apr 4, 2018

@feross is it time? 😎

@feross

This comment has been minimized.

Copy link
Member

commented May 15, 2018

Yep, I think it's time 👍

@feross feross reopened this May 15, 2018

@feross feross added the enhancement label May 15, 2018

@feross feross added this to the standard v12 milestone May 15, 2018

@feross feross modified the milestones: standard v12, standard v13 Aug 28, 2018

@harrysarson

This comment has been minimized.

Copy link

commented Mar 9, 2019

May I inquire about the status of this one?

@LinusU

This comment has been minimized.

Copy link
Member

commented Mar 11, 2019

This is something that we want to get in, if you want to work on this I would start with compiling a list of all the repos that would break if we change this. If it's not too many, we can open a PR against eslint-config-standard and start sending PRs to upstream projects (e.g. as in this one)

You can get the list by cloning this repo, adding the rule to eslintrc.json, and running the tests.

@feross

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

Anyone want to help start the process of opening PRs against repos that would break? I want to get this into standard v14

@sonicdoe sonicdoe referenced this issue Aug 2, 2019
29 of 152 tasks complete
@sonicdoe

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

I’d like to help with this 👍 I’ve opened standard/eslint-config-standard#152 to add no-var to the ESLint config and included a list of all failing repositories in the description.

@feross

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

@sonicdoe Thanks for taking on this task! 💪

@feross feross modified the milestones: standard 14, standard 15 Aug 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
8 participants
You can’t perform that action at this time.