Update linter to work with ESLint 8

[cjihrig]

Support plan

• is this issue currently blocking your project? (yes/no): not yet
• is this issue affecting a production system? (yes/no): not yet

Context

• node version: v16.8.0
• module version: v24.3.2
• environment (e.g. node, browser, native): node
• used with (e.g. hapi application, another framework, standalone, ...): standalone
• any other relevant information:

What problem are you trying to solve?

ESLint 8 is currently in beta. It would be good to start migrating to ESLint 8 before it goes stable because there are some updates to be made in Lab. Lab uses ESLint's CLIEngine class, which has been removed in ESLint 8.

Do you have a new or modified API suggestion to solve the problem?

Update Lab's ESLint according to the migration guide.

[Nargonath]

I looked a bit into it when I migrated lab to our new ESLint plugin but this might imply big changes hence I didn't handle it in my previous PR. We go from a synchronous API for some functions to an async one i.e getConfigForFile vs calculateConfigForFile.

[cjihrig]

FYI - ESLint 8.0.0 has been released.

[geek]

It looks like a breaking change was to remove the CLIEngine from the exports: eslint/eslint@24c9f2a#diff-3814d8caba69e5fdf3b23eed56ec7d708b8ede5aed6d686ac579421877938ec7 The CLIEngine is also not exposed via the package.json exports, therefore we likely won't be able to access it directly without creating a new ESLint.

[cjihrig]

The CLIEngine has been replaced. You can take a look at cjihrig/belly-button@beaf30a, which might help the migration process.

[geek]

Thanks @cjihrig ... I've started the work here: geek@0ea7ce4 looks like a similar experience with adding in more async functions

[geek]

We may also need to wait for babel/babel#13782 to land

[cjihrig]

@geek does cjihrig/belly-button@4a151b3 make any difference? I was able to get ESLint 8 running.

[cjihrig]

Ah, that's related to transpiling :-/

[cjihrig]

babel/babel#13782 has landed and apparently released: https://babeljs.io/blog/2021/10/29/7.16.0

[geek]

@cjihrig thanks, yeah I started incorporating it last weekend, but ran into some issues. Hoping to have a PR next week.

[geek]

I ran into an issue with https://github.com/nodejs/node/blob/873119385f4b59d9a0df75284790203102054ea1/lib/internal/modules/cjs/loader.js#L975 assuming a sync function, which we assign at

lab/lib/modules/coverage.js

Line 119 in e7b0550

require.extensions[extension] = function (localModule, filename) {

With the async change in eslint for calculateConfigForFile this will require instrument to become async as well as this "extensions" function. Fortunately, the calculateConfigForFile doesn't need to be async, so I submitted a PR for that change here: eslint/eslint#15275 We will see what the eslint folks think of the change.

[Nargonath]

Good catch @geek. I didn't check whether it was purposefully async or not. I assumed it was.

[geek]

@cjihrig I don't see a way to make this work. If I try to load the cli engine directly it throws ERR_PACKAGE_PATH_NOT_EXPORTED due to not being part of the official eslint export.

[devinivy]

I took a look at this, and I agree it's not pretty! My recommendation is to call calculateConfigForFile() once for Path.join(coveragePath, 'x') (the file doesn't actually need to exist), then use the resulting parserConfig for all file coverage. This is a breaking change and a limitation, but I think it's pretty reasonable/workable to only support one parser config for coverage purposes.

[devinivy]

There is a PR implementing the suggestion in my previous comment here: #1035