fix: Detect ESLint >= 8 and tell the user about linter-eslint-node

[savetheclocktower]

The new linter-eslint-node package is the way forward, but most users won’t know about it unless we tell them. This PR proposes one way we could do that.

Approach

If I try to use linter-eslint in an ESLint v8 project, I can trace the problem down to this particular load error:

Without knowing exactly why it’s happening, I'm going to assume that's an artifact of trying to load ESLint v8 in an environment that it officially doesn’t support (going by its engines field in package.json).

I don’t want to check the ESLint version every time we try to load it, only when we think that ESLint might be too new. So let’s check in these two scenarios:

1. If require('eslint') throws an error.
2. If require('eslint') somehow works but doesn’t give us an object with a CLIEngine property.

I don’t want to try to inspect the error; the fact that an error happened is excuse enough to read the package.json and check the value. All other code paths work just as they did before.

On the package side, we receive this IncompatibleESLintError the same way we’d receive any other error thrown by the worker, but I’ve got the worker sending along workerErr.name so we can tell them apart more easily.

The typical way of showing worker errors is via the results console, but I wanted this one to show as a notification so that the user has an easy way to install the new package. handleError checks for this specific kind of error and calls a function that will show that notification if (a) the user hasn’t already seen it this session, and (b) the user doesn’t already have linter-eslint-node installed and active.

Open questions

• Do we want to give users a setting for suppressing the new notification altogether? I don’t know how many users will want to preserve the status quo without either downgrading their ESLint or installing the new package, but if there are any at all, I can imagine them getting annoyed at a warning that they plan to dismiss every time.

• When linter-eslint and linter-eslint-node coexist, and are both active in a project using ESLint v8, linter-eslint will do a lot of work before deciding to fail silently. On every lint, it’ll try to load the too-new ESLint, fail, read the package.json, throw the error… et cetera. I don’t think this matters much for performance, but it is 99.9% certain to be unnecessary work. Do we want to cache this decision to prevent some of this work? If so, is there any scenario in which we'd need to invalidate that cache, or are we OK with persisting that decision until the user reloads the window or opens the project again?

[savetheclocktower]

linter-eslint-node#12 made me realize that it's probably worth changing this package's description at the same time, so I added that as well.

[UziTech]

Do we want to give users a setting for suppressing the new notification altogether? I don’t know how many users will want to preserve the status quo without either downgrading their ESLint or installing the new package, but if there are any at all, I can imagine them getting annoyed at a warning that they plan to dismiss every time.

I think we should add a setting for linter-eslint to just ignore v8 and not warn. I could see that being desired by a user.

When linter-eslint and linter-eslint-node coexist, and are both active in a project using ESLint v8, linter-eslint will do a lot of work before deciding to fail silently. On every lint, it’ll try to load the too-new ESLint, fail, read the package.json, throw the error… et cetera. I don’t think this matters much for performance, but it is 99.9% certain to be unnecessary work. Do we want to cache this decision to prevent some of this work? If so, is there any scenario in which we'd need to invalidate that cache, or are we OK with persisting that decision until the user reloads the window or opens the project again?

I think we can start with just invalidating cache on reload that should handle 99% of situations.

[savetheclocktower]

OK, addressed both pieces of feedback. I've got a boolean that starts out false and flips to true the first time we encounter an incompatible version in a given window, and immediately kills the worker and short-circuits lint and fix jobs.

The new option is under “Uncommon” but can be moved somewhere else if you like.

[UziTech]

Can you get the tests passing?

[savetheclocktower]

Got them passing locally, but the workflow needs approval… again? That's weird. Never mind, there they go.

Had to restructure so that the tests didn't fail our new inspection logic, and took the opportunity to write a similar test that ensures an ESLint without a CLIEngine property will check the version and throw.