eslint / eslint

Find and fix problems in your JavaScript code.
https://eslint.org
MIT License
24.39k stars 4.4k forks source link

Bug: An error occurs when invoking `ESLint#lintFiles()` in Jest #18407

Closed StewEucen closed 1 week ago

StewEucen commented 2 weeks ago

Environment

Node version: 20.12.2 npm version: 10.5.0 Local ESLint version: 9.1.1 Global ESLint version: 9.1.1 Operating System: macOS

What parser are you using?

Default (Espree)

What did you do?

Configuration * eslint.config.js ```js 'use strict' module.exports = [ { languageOptions: { parserOptions: { ecmaVersion: 'latest', }, }, ignores: [ '**/node_modules/**', ], rules: { indent: ['error', 2], quotes: ['error', 'single'], semi: ['error', 'never'], }, }, ] ```

What did you expect to happen?

What actually happened?

Link to Minimal Reproducible Example

https://github.com/StewEucen/eslint-bug-with-jest

Participation

Additional comments

mdjermanovic commented 2 weeks ago

I can reproduce this locally. Thanks for the detailed analysis of the cause!

https://github.com/StewEucen/eslint-bug-with-jest?tab=readme-ov-file#cause

  • Retrier#retry() uses Promise declared in lib.es2015.iterable.d.ts
  • fs.readFile() uses Promise declared in lib.es5.d.ts

So it seems that in Node.js different operations may use different Promise constructors?

@nzakas perhaps @humanwhocodes/retry could check if the result is a thenable instead of checking result instanceof Promise?

StewEucen commented 2 weeks ago

@mdjermanovic


So it seems that in Node.js different operations may use different Promise constructors?

  • I think that since fs is old package, it uses polyfill Promise class.

fasttime commented 2 weeks ago

The Promise object returned by fs.readFile is from a different realm:

image

This only occurs when the Node.js process is launched with the --experimental-vm-modules flag. It may have something to do with Jest's experimental ESM support, which is enabled by the flag.

StewEucen commented 2 weeks ago

@fasttime


  1. I had used ESLint#lintFiles() in Jest with ESLint@8.x.x. As we know, Jest works as CommonJS. But It did not require to use --experimental-vm-modules flag.

  2. I changed the code to fit Flat Config with 9.1.1, but it was not work. Because new code of ESLint#lintFiles() uses dynamic import.

    https://github.com/eslint/eslint/blob/v9.1.1/lib/eslint/eslint.js#L317

    async function loadFlatConfigFile(filePath) {
        ...
    
        const config = (await import(fileURL)).default;
    
        ...
    }

    https://github.com/eslint/eslint/blob/v9.1.1/lib/eslint/eslint.js#L369

    async function calculateConfigArray(eslint, { ... }) {
        ...
    
        if (configFilePath) {
            const fileConfig = await loadFlatConfigFile(configFilePath);
    
            ...
        }
    
        ...
    }

    https://github.com/eslint/eslint/blob/v9.1.1/lib/eslint/eslint.js#L815

    
    class ESLint {
        ...
    
        async lintFiles(patterns) {
            ...
    
            const configs = await calculateConfigArray(this, eslintOptions);
    
            ...
        }
    
        ...
    }
  3. To solve the problem, I added a --experimental-vm-modules flag for Jest script.

    https://github.com/StewEucen/eslint-bug-with-jest/blob/main/package.json#L7

    {
      "scripts": {
        "test": "export NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules\"; jest --forceExit"
      },
    }
  4. And then, I got the behavior that I reported in this issue.

nzakas commented 2 weeks ago

I can update retry. Never occurred to me that there might be a promise from another realm involved.

nzakas commented 2 weeks ago

A new version of @humanwhocodes/retry was released as a patch version that should fix this.

mdjermanovic commented 2 weeks ago

Here's a PR to update the dependency: https://github.com/eslint/eslint/pull/18416

StewEucen commented 2 weeks ago

@nzakas @mdjermanovic @fasttime