feat: Support new ESLint engine

[scagood]

Description

This is my first pass attempt to support eslint v8 (#1440).

I have made this PR because I have hit 2 road blocks:

1. Plugins dont seem to be loading. I just get:

Failed to load plugin '{plugin-name}' declared in '{project-path}/.eslintrc.js'

2. Ignoring all fixable rules while typing. I cant see how to get working as the api has just directly been removed (see: https://eslint.org/docs/user-guide/migrating-to-8.0.0#the-cliengine-class-has-been-removed)

Any help fixing these two problems would be greatly appreciated! (I just need to be pointed in roughly the correct direction if that is possible)

[UziTech]

I think the issue is that Atom uses an old version of node (12.14.1) and I don't think it has the ability to load es modules which eslint plugins are starting to use.

[scagood]

I have disabled these tests because as far as I can see there is no way to detect what rules are loaded and fixable for a given file 😬

[brettz9]

What about ESLint#calculateConfigForFile?

[brettz9]

FWIW, although Node 12 did start ESM support, ESLint required a higher minor version per this comment eslint/eslint#14023 (comment) .

It seems Atom is really slow to update the bundled Node version due to being deterred from Electron upgrades (e.g., see atom/atom#22765 ). Even the Electron version on master, 11.4.12 corresponds only to Node 12.18.3 which is behind ESLint 8's minimum of 12.22.0.

Though less than ideal, how feasible would it be to bundle a higher Node version and use that in its own process or what not? It seems it may be a while otherwise, and not supporting the latest ESLint, especially after some additional minor releases already having occurred, really does not seem viable for a modern IDE to keep apace.

[UziTech]

One way we could get around the node version issue would be to run eslint cli and parse the output.

[scagood]

I have actually just stopped trying to adapt this (because supporting the options was rather hard). I instead wrote my own version from scratch here is the code if you want it in a gist (https://gist.github.com/scagood/061ebc869de679abc1324ab184cdd4a0).

It does not have any options because I couldnt be bothered to add to something I was not sure I was going to share it.

There is one major issue in the one I wrote, that being it assumes that 'node' is executable from atom (which it wont be if you use nvm).

[gjermundnor]

I have actually just stopped trying to adapt this (because supporting the options was rather hard). I instead wrote my own version from scratch here is the code if you want it in a gist (https://gist.github.com/scagood/061ebc869de679abc1324ab184cdd4a0).

Thanks @scagood! Your script solved it for me 🙏

FYI:
I'm using Atom, and first got the error "Node cannot be spawned. Is it installed and on your PATH?".
Node is on my PATH, but only available through the shell. Without enabling the PATH outside the shell I manged to get it working by just opening Atom from the shell instead. Like this: atom .

[scagood]

I'm using Atom, and first got the error "Node cannot be spawned. Is it installed and on your PATH?". Node is on my PATH, but only available through the shell. Without enabling the PATH outside the shell I manged to get it working by just opening Atom from the shell instead. Like this: atom .

🤔 I guess you're using nvm or something similar?

@UziTech Do you want me to PR the code I linked, which drops support for eslint < 6 and removes most of the options?

[UziTech]

@scagood it might be better to create a different linter with your code so we don't break people who use older eslint and need the options.

If you want we could host it in AtomLinter.

[brettz9]

Yes, please, would be great if this could be released even as a new and less-featured package. I think Atom itself really needs this.

[savetheclocktower]

I like the approach that @scagood's gist takes: instead of consuming ESLint's JS API within Atom's own version of Node, consume it with whatever version the project is using, and run our own worker script within that environment, passing messages back and forth between the two processes.

I'm stuck on how to reliably detect which version of Node the user needs, and I've been trying to think through it. When I wrote an ESLint bundle for TextMate many years ago, I could just rely on the user to tell me which one to use by defining a TM_ESLINT variable either in their project config or their global config (or both, with the former overriding the latter). With no built-in support for per-project settings, Atom doesn't have it so easy.

Here are the options that I can think of — I know someone else will be able to think of more:

1. Throw a bunch of heuristics at it. If the user started Atom from the shell, process.env.NVM_DIR (or even just the output of which node) will tell us where Node is. If they started it a different way, check for the presence of an .nvmrc file in the project root. (Though that's not enough to get the full path to the executable; there'd also have to be a setting for “nvm root path” or something so that the linter could build a path to the correct Node from parts). As a final fallback, the linter could just treat which node as authoritative.
2. Give up on the worker and run eslint directly via CLI. The presence of ./node_modules/.bin/eslint will tell us pretty unambiguously that the user wants the project's version of ESLint to be used, and I believe this will work whether the user prefers Yarn or NPM. This should mean that the linter no longer needs to track down the correct version of Node to use. The tradeoff here is that the linter integrates via CLI output (eslint --format json, or perhaps eslint --format json-with-metadata), rather than having the entirety of the library API available, but I don't imagine that's a huge problem — most linter packages seem to integrate this way.
3. There's a way to get the advantages of both option 1 (consuming the API via a worker script) and option 2 (checking for a single binary instead of complicated find-the-right-Node heuristics): the linter could put the worker script into an NPM package and make that a prerequisite for Atom integration. The drawbacks of this approach are obvious, and it's my least favorite of all these ideas, but I'm mentioning it in case it produces a better idea.

This saga has made me curious about whether other packages have had this problem, and how they've solved it. But I can't find anything obvious, plus it's not the easiest thing to google.

[savetheclocktower]

I just discovered I'm wrong about Option 2: I figured the eslint binary would be unambiguous about which version of Node it needed, but it's got #!/usr/bin/env node as its shebang, so there's no easy way out of the Node thing. Running an eslint binary for version 8 will presumably fail if which node finds a version of Node that doesn't grok ES modules. Ugh.

[brettz9]

I just discovered I'm wrong about Option 2: I figured the eslint binary would be unambiguous about which version of Node it needed, but it's got #!/usr/bin/env node as its shebang, so there's no easy way out of the Node thing. Running an eslint binary for version 8 will presumably fail if which node finds a version of Node that doesn't grok ES modules. Ugh.

Yes, ESLint 8 itself dropped support for Node before version 12 and specifically requires at least 12.22/14.17/16(see eslint/eslint#14023 though see esp. eslint/eslint#14023 (comment) on the exact versions or https://github.com/eslint/eslint/blob/main/README.md#installation-and-usage for confirmation in the docs.)

Not sure how important is it to support older versions of Node with those versions having been end-of-lifed, and much other modern tooling has dropped older Node support as well, but if the reason is to allow switching to nvm for testing of packages which support older versions, I still think a broken linter for that scenario would be superior to the present situation of it never being able to work even under the more common scenario of using a non end-of-lifed Node version.

If you really need it though, could you check the package.json of eslint for its engines field and then parse it with the semver npm package to find at least the minimum version within that range and then perhaps iterate the installed nvm versions? Sorry if I'm not following exactly all you're trying to do.

[savetheclocktower]

The problem is this, @brettz9 (and whoever else is curious; I'll over-explain just to get all possible readers on the same page):

Imagine you've got a local NPM project where you've installed ESLint 7. The linter-eslint package will integrate with your version of ESLint (and your config file) by finding what you've installed in node_modules and calling require('eslint') directly. From there it uses the CLIEngine object that ESLint exposes and lints your code that way.

Now you upgrade to version 8 and your linting has stopped working. You think it's because ESLint 8 removed CLIEngine — and they did — but that's the least of your problems, because they also started using ESM imports instead of require imports, and Atom's version of Node can no longer require('eslint'). One option is for Atom internals to use a newer version of Node, but they can't, because not even Electron bundles a version of Node new enough to use modules, and nobody knows when they will.

To be honest, this was always a bit of a hack, and could theoretically have broken at any point in the past, since there's no guarantee that a random package pulled from a user's own environment would work with Atom's specific version of Node. So it's safer to switch to a model where Atom calls a process and that process runs the linting and reports back the results, because then that process isn't bound to Atom's old version of Node.

The best version of Node to use is whichever one the user was using when they ran npm install eslint for that project. That's the one that's guaranteed to work, and it's probably the version that would run if they typed node in a shell inside their project root today. But it's not obvious which version that is because we don't have access to the shell environment as defined by the user. We don't know what they've set their PATH to; maybe /usr/bin/node exists but the user has installed node into /usr/local/bin/node, which we wouldn't necessarily find. Running, say, which node from a child process might not find any Node, much less the one we need.

That's not even getting into the NVM-specific stuff! To iterate the installed NVM versions, as you suggest, would require access to the nvm command, which isn't even a binary; it's a shell function that gets sourced via someone's .profileor .bashrc or what-have-you. On Linux and macOS, all this would work perfectly if we knew that the project had been launched via the atom CLI tool, because then Atom will inherit the environment variables of the shell that invoked it, and we could just read $NVM_BIN. To my utter astonishment, this seems to work on a per-window basis! I've got a few Atom windows open for projects that use different versions of Node, and process.env.NVM_BIN in each window's console returns the correct value for each one.

But that's just because I typically open my project by cding into my project folder and running atom .. If I close one project window and then re-open it by dragging a Finder folder onto the Atom icon, or doing File → Reopen Project, process.env.NVM_BIN is present but points to the wrong thing — maybe the value from the next-most-recently-opened window or something. At any rate, it can't be relied upon.

$NVM_DIR can be relied upon if present, since it'll point to the same thing across all windows. But that's the root directory of the NVM installation. You can get to each version's binary from there, but how do you find the right version? You can traverse upwards from the project root for .nvmrc files, but you're not guaranteed to find one; maybe the user doesn't define one when a project is using their default version of Node.

Also, maybe the user prefers Volta. I hear it's quite good.

The way to cut this Gordian knot is to ask the user to specify the path to the right version of Node, but that can vary from project to project, and Atom has no built-in concept of per-project settings. So I have no idea what to do. I can think of solutions which would mostly work for me, but not necessarily for other macOS users, much less Windows users (lord, how does any of this work on Windows?).

[brettz9]

Thanks for the info... Plan to look over more carefully, but although ESLint is moving toward ESM support, with some of its dependencies offering such exports--but those are in addition to CommonJS, and ESLint itself actually doesn't even offer ESM itself. It only offers CommonJS still. The new plugin format--which I have not yet gotten into using yet myself--was supposed to perhaps support reading of ESM plugins, but I haven't confirmed that myself.

Perhaps the issue was that ESLint is using exports in package.json which no longer allows grabbing other files besides the approved ones, or maybe some other interaction as with plugin loading changes. But the binary is CommonJS style, and even after adding ESM support, I would expect we'd be continuing to support CJS.

[savetheclocktower]

Yeah, I oversimplified — some plugins are starting to use ES modules, not ESLint itself. @scagood hit his roadblock above when he found that the plugins he was testing with weren't loading.

I looked into how VSCode's ESLint extension handles this, and it's as I suspected. Since VSCode supports per-project settings, the user can define editor.runtime and editor.nodePath settings globally as a fallback (if they've got an ESLint package installed globally) and use per-project versions of the same settings to define exceptions as needed.

I am hesitant to propose new third-party dependencies to fix problems, but I'll bring this up anyway: project-config hooks into plumbing that already exists in Atom to make per-project settings possible. (It doesn't have a lot of existing users, but neither does any other Atom package that didn't already exist five years ago.) Perhaps the answer could be “use this setting to tell us where Node is, and install this other optional package if that setting needs to change from project to project.”

[brettz9]

Perhaps the answer could be “use this setting to tell us where Node is, and install this other optional package if that setting needs to change from project to project.”

If so, I also hope using the approach of ./node_modules/.bin/eslint could still be considered as an option. Would really be cumbersome I think if switching nvm versions causes breakage and needing project config or digging into settings when the executable was right there. Also, I personally tend to switch nvm versions for the sake of testing locally, including ESLint, and would ideally have the current Node version used as a result of execution of the ESLint executable.

Thanks for all your helpful explanations, by the way. And would be great in any case, to see something allowing ESLint 8 access again!

[savetheclocktower]

If so, I also hope using the approach of ./node_modules/.bin/eslint could still be considered as an option. Would really be cumbersome I think if switching nvm versions causes breakage and needing project config or digging into settings when the executable was right there. Also, I personally tend to switch nvm versions for the sake of testing locally, including ESLint, and would ideally have the current Node version used as a result of execution of the ESLint executable.

Sadly, I was wrong about that; since the shebang line of that eslint binary is #!/usr/bin/env node, it will try to run with the first version of Node found in PATH, so it doesn't save us from having to figure out the right Node.

Would really be cumbersome I think if switching nvm versions causes breakage and needing project config or digging into settings when the executable was right there.

I agree, but I think that NVM would need to meet us halfway here; this is the downside of having a version manager that is so heavily reliant on shell voodoo. In theory, NVM could create its own binary that, when invoked, would traverse upward from CWD to find an .nvmrc file, and use the first one it finds, falling back to whichever version was set to be the global default. That would solve most use cases, though not the one you mention — if you've got an .nvmrc with contents bar in some directory, you can cd into that directory, run nvm use foo — and then your Node version is foo in that shell only, without a good way for stuff outside of that shell to understand your intent.

This is mainly why I moved from RVM to rbenv way back when, and sometimes lessons need to be re-learned.

[amxmln]

The way to cut this Gordian knot is to ask the user to specify the path to the right version of Node, but that can vary from project to project, and Atom has no built-in concept of per-project settings. So I have no idea what to do. I can think of solutions which would mostly work for me, but not necessarily for other macOS users, much less Windows users (lord, how does any of this work on Windows?).

Would users adding a .linter-eslint file with the path to their projects be an option to have a cross-plattform-per-project way of telling the plugin? I’m not familiar with how exactly Atom plugins work, but while not being the best UX, it seems like a simple enough workaround to at least have ESLint working in Atom again.

[savetheclocktower]

That's an option, sure, and maybe it's a way forward for people who don't want to install the project-config package. My vote would be to support both, since VSCode has normalized concept of per-project config; hypothetically, if other packages were to need their own per-project config settings, I'd want those all to live in the same place rather than have each package choose some random dotfile to stick their settings into.

Whereas project-config would want a file that could be directly applied over the default config file:

{
  "linter-eslint-v8": {
    "nodeBin": "/Users/andrew/.volta/bin/node"
  }
}

…a .linter-eslint file should just have overrides specific to the one package:

{
  "nodeBin": "/Users/andrew/.volta/bin/node"
}

Only snag is what happens when both configs are present and disagree. The answer could just be to pick an order in which they're consumed and stick with it; I don't think it matters which order, as long as it's consistent.