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

new rule: force a variable to be a constant if not used with assignment operator anywhere #396

Closed
capaj opened this issue Jan 31, 2016 · 11 comments

Comments

@capaj
Copy link
Contributor

commented Jan 31, 2016

This rule should only be enforced for node.js > 0.10

There is no point in declaring a module as var in newer versions of node. Module should always be a const if assigned on the same line as the require statement is. If you want to require module "A" or module "B" into a single variable, it should be done like this:

var aOrB
if (cond) {
  aOrB = require('A')  
} else {
  aOrB = require('B')
}

if you would have do it like this:

var aOrB  = require('A')  
if (cond) {
  aOrB = require('B')
}

That would be bad, because that way you are forcing node to load module "A" even if cond is true.

Thanks for you consideration and please let me know if I forgot some usecase where it might be valid to use var.

@watson

This comment has been minimized.

Copy link
Member

commented Feb 1, 2016

There is no point in declaring a module as var in newer versions of node

Can you please elaborate on why this is? I don't mean to put down your suggestion, I'm just not familiar with why this is 😃

@capaj

This comment has been minimized.

Copy link
Contributor Author

commented Feb 1, 2016

@watson http://ideasintosoftware.com/const-is-the-new-var/
Basically you should only use let/var when you actually want to reassign it. In the case of node.js require()s, reassigning them makes very little sense. Making them immutable references will be a nice protective harness for noobs not to do crazy things in their JS. It also helps in my editor- const are coloured differently, so the code becomes more readable.

@dcousens

This comment has been minimized.

Copy link
Member

commented Feb 2, 2016

NACK

We're better off just enforcing import X from 'Y' when we hit ESNEXT.
This would be frustrating and rarely useful behaviour.

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2016

agree with @dcousens' reasoning

@watson

This comment has been minimized.

Copy link
Member

commented Feb 2, 2016

Personally I've yet to see a situation where a bug was introduced because someone used var when requiring a module. Isn't this in the same category as the "let's use semicolons because it's safer" argument? In theory, yes... you could by accident overwrite the variable - but have anyone ever accidentally done so?

Also, this will make standard unusable when creating code that need to work with versions of Node or browsers that doesn't support const.

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2016

but have anyone ever accidentally done so

Oh yeah, I definitely have haha 😅

@capaj

This comment has been minimized.

Copy link
Contributor Author

commented Feb 2, 2016

Also, this will make standard unusable when creating code that need to work with versions of Node or browsers that doesn't support const.

True, that is why this rule should be released in a breaking version like 6.0.0 or 7.0.0 and support for node.js < 0.12 dropped from standard all together. Users on node.js 0.10 and less can still use 5.x.x

Now that I think about it, it would be better if we had a rule which could check whether you're assigning into a var/let. If you're not, then it should be a style error of something like: No assignment to variable X - constant should be used instead

That would cover much more potential issues.

@rstacruz

This comment has been minimized.

Copy link
Member

commented Feb 2, 2016

standard supports es5 code, and this would be a breaking change. perhaps this is better suited for something like standard-es2015?

@capaj

This comment has been minimized.

Copy link
Contributor Author

commented Feb 2, 2016

@rstacruz I think standard is already supporting features outside of es5, so why start another "standard" now?

@capaj capaj changed the title new rule: forbid var module = require('mymodule') new rule: force a variable to be a constant if not used with assignment operator anywhere Feb 2, 2016

@rstacruz

This comment has been minimized.

Copy link
Member

commented Feb 2, 2016

because adding a rule like this will make standard unusable for any code that's targeting legacy node.js versions or browsers (without babel pre-compilation).

@capaj

This comment has been minimized.

Copy link
Contributor Author

commented Feb 2, 2016

@rstacruz yeah browsers would be a problem. Hmm ok then.
aaa

@capaj capaj closed this Feb 2, 2016

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

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