Support separate requires in one-var

[andrewvaughan]

ESLint Version: 2.10.1

Problem:

A common request (e.g, #3091, #1232) is for the one-var rule to properly function in tandem with no-mixed-requires. I agree with this opinion, as the two methods are part of the core set of rules provided with ESLint and should properly support each other. I also believe there is a way to support this without introducing a dependency between the two rules.

Recommended Solution:

I believe this should not be a plugin, as both commands are part of the core library and cause conflict with each other. A better solution would be to add an optional parameter separate-requires to one-var that allows for require keywords to be separated into a single block, while assignments are in another block (similar to the intent of the two-var plugin).

For example:

"one-var": ["error", {
    "separate-requires": true,  // (New) Enforces requires to be separate from declarations
    "var": "never",
    "let": "never",
    "const": "never"
}]

This will allow ESLint to properly support it's built-in plugins without changing the default functionality of the one-var rule. With the above configuration:

This code would result in an error:

const gulp = require('gulp'),
      myModule = require('./some/module'),
      myVar1 = 1,
      myVar2 = 2;

This code would be valid:

const gulp = require('gulp'),
      myModule = require('./some/module');

const myVar = 1,
      myVar = 2;

[nzakas]

I'm not in favor of adding a CommonJS-specific option to a general rule, so I'm 👎 On this suggestion.

[nzakas]

Looks like there's already a lengthy discussion of this here: #4680

So closing as a duplicate.

[mysticatea]

I reopened this since this matter was separated from #4680.

[nzakas]

@mysticatea is this part of JSCS work?

[mysticatea]

Yes, requireMultipleVarDecl seems to have an exception for require().

[kaicataldo]

Having a little trouble tracing all the issues about this - where does this stand, @mysticatea? Is the original proposal still what we want to implement? If so, I'll champion this.

Edit: I think the option name should be camel case since it's an object key: separateRequires

[kaicataldo]

@mysticatea Friendly ping

[mysticatea]

I'm sorry for too late!

I think, yes.
JSCS has the option to ignore variable declarations which have require().

(Though, it seems ignoreRequires-like behavior rather than separate.)

[kaicataldo]

(Though, it seems ignoreRequires-like behavior rather than separate.)

Right. Honestly, not sure how to proceed with this. I think your suggestion makes more sense, but it is different than what the JSCS option does. Seems like what seperateRequires enforces would be the common use case for it though.

[kaicataldo]

@eslint/eslint-team Thoughts on this? Had a request for this in the Gitter chat today and would like to support this for those who are converting from JSCS.

The big remaining question is whether we track the JSCS ignore behavior or implement the separate require behavior proposed here. I personally like the latter, as I think it's more in line with the spirit of the rule and because it seems like that's the intended use case for the ignore option in JSCS.

[not-an-aardvark]

I agree, having an option to separate require behavior sounds better than just having an option to ignore require.

[vitorbal]

👍 on this require behavior instead of just plain ignoring.

[nzakas]

Can we get some JSCS folks input here?

[mikesherov]

I'm for the JSCS behavior, considering it's more lenient, makes switching from JSCS easier, and we can always add the stricter version later.

That said, I'm fine either way.

[kaicataldo]

@eslint/eslint-team Sounds like we have 4 (including myself) in favor of the separate require option being proposed here and 1 to port over the JSCS behavior. Any other input? Would be nice to come to some consensus on this so we can get the ball rolling on actually implementing this.

[mikesherov]

Don't let my opinion hold this back from moving forward.

[mikeletscher]

I will take a look into getting a PR up for this sometime this week/weekend.

[kaicataldo]

@mikeletscher Fantastic! Please feel free to visit us in the ESLint Gitter if you get stuck or want to chat!

[onurtemizkan]

@mikeletscher are you still working on this? I'm planning to make a PR this weekend if it's ok for you.

[mikeletscher]

@onurtemizkan Was like 2/3 of the way done, but don't let me stop you!

[onurtemizkan]

@mikeletscher No! Please go for it ☺️

[mikeletscher]

Hey guys, I dropped the ball on this--didn't end up having the time to finish it. @onurtemizkan I don't want to block you if you wanted to get this fixed, sorry for following up so late!