Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to use ESLint flat config for linting JavaScript #3570

Open
theetrain opened this issue May 22, 2024 · 20 comments
Open

Unable to use ESLint flat config for linting JavaScript #3570

theetrain opened this issue May 22, 2024 · 20 comments
Labels
bug Something isn't working

Comments

@theetrain
Copy link

Describe the bug

I tried pointing MegaLinter to my eslint.config.js, but it yielded the error:

❌ Linted [JAVASCRIPT] files with [eslint]: Found 1 error(s) - (1.49s)
- Using [eslint v8.57.0] https://megalinter.io/7.11.1/descriptors/javascript_eslint
- MegaLinter key: [JAVASCRIPT_ES]
- Rules config: [/eslint.config.js]
- Number of files analyzed: [20]
--Error detail:
Invalid option '--eslintrc' - perhaps you meant '--ignore'?
You're using eslint.config.js, some command line flags are no longer available. Please see https://eslint.org/docs/latest/use/command-line-interface for details.

To Reproduce

  1. Add these settings to .mega-linter.yml
JAVASCRIPT_ES_CONFIG_FILE: ./eslint.config.js
  1. Run megalinter

Expected behavior

Flat ESLint config file is picked up, and runs without error.

@theetrain theetrain added the bug Something isn't working label May 22, 2024
@ethanchristensen01
Copy link

I tried getting around this by removing the --eslintrc option.

# .mega-linter.yml
# ...other configs
JAVASCRIPT_ES_CONFIG_FILE: ./eslint.config.mjs
JAVASCRIPT_ES_COMMAND_REMOVE_ARGUMENTS: ["--eslintrc"]

When I run the linter with this configuration, I get this error:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/megalinter/run.py", line 14, in <module>
    linter.run()
  File "/megalinter/MegaLinter.py", line 245, in run
    self.process_linters_serial(active_linters)
  File "/megalinter/MegaLinter.py", line 297, in process_linters_serial
    linter.run()
  File "/megalinter/Linter.py", line 793, in run
    return_code, stdout = self.process_linter()
                          ^^^^^^^^^^^^^^^^^^^^^
  File "/megalinter/Linter.py", line 936, in process_linter
    command = self.build_lint_command(file)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/megalinter/linters/EslintLinter.py", line 12, in build_lint_command
    cmd = super().build_lint_command(file)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/megalinter/Linter.py", line 1286, in build_lint_command
    cmd.remove(arg)
ValueError: list.remove(x): x not in list

@ethanchristensen01
Copy link

I found that replacing --eslintrc with --no-eslintrc fixed that error, but I got another one:

- Using [eslint v8.57.0] https://megalinter.io/7.11.1/descriptors/javascript_eslint
- MegaLinter key: [JAVASCRIPT_ES]
- Rules config: [/eslint.config.mjs]
- Ignore file: [/.eslintignore]
- Number of files analyzed: [305]
--Error detail:
Invalid option '--ignore-path' - perhaps you meant '--ignore-pattern'?
You're using eslint.config.js, some command line flags are no longer available. Please see https://eslint.org/docs/latest/use/command-line-interface for details.

It seems javascript.megalinter-descriptor defines the ignore option to be --ignore-path, which doesn't work for flat config.

--ignore-pattern is valid for both configs, but it requires a list of globs instead of an ignore file.

@echoix
Copy link
Collaborator

echoix commented May 27, 2024

Is the flat config supported before version 9 of eslint? I see the last release of Megalinter had eslint 8.57.0.

@ethanchristensen01
Copy link

Yes, the eslint website has a migration guide which mentions the flat config is the default in eslint 9, but is also available in eslint 8.

@echoix
Copy link
Collaborator

echoix commented May 27, 2024

In that migration guide, I read that using an ignore file isn't supported anymore. That would explain why --ignore-path doesn't work. They say to

https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files

The old way of ignoring (what Megalinter is still set up for), has docs here: https://eslint.org/docs/latest/use/configure/ignore-deprecated

You can try to extrapolate what config is created by the linter's descriptor here (or use debug logging if possible to see what would be called):

active_only_if_file_found:
- ".eslintrc.json"
- ".eslintrc.yml"
- ".eslintrc.yaml"
- ".eslintrc.js"
- ".eslintrc.cjs"
- "package.json:eslintConfig"
cli_lint_mode: list_of_files
config_file_name: .eslintrc.json
cli_config_extra_args:
- "--no-eslintrc"
ignore_file_name: .eslintignore
cli_lint_ignore_arg_name: --ignore-path
cli_lint_extra_args:
- "--no-ignore"
cli_lint_fix_arg_name: "--fix"
cli_sarif_args:
- --format
- "@microsoft/eslint-formatter-sarif"
- -o
- "{{SARIF_OUTPUT_FILE}}"
cli_lint_errors_count: regex_sum
cli_lint_errors_regex: "✖ ([0-9]+) problem"

And finally, at the end of all this, I'd like to know your experience on if it would be possible to support both current config and flat config in Megalinter, or we need to force all users to use v9 and migrate their configs+project configs and those who can't would need to use an older Megalinter. Since the eslintignore doesn't exist anymore, is there a way we could have a working base preset/config for users that don't have a config yet?

@ethanchristensen01
Copy link

It depends on what we can do in EslintLinter.py

ESlint v8 and v9 still support both formats, the default just changed between the two versions. We can set an environment variable to override it (ESLINT_USE_FLAT_CONFIG=false).

Figuring out which format a user wants could be done in a few ways:

  • Define a new config variable, e.g. XYZ_ES_USE_FLAT_CONFIG
  • Extrapolate whether a config is flat or not based on its name from XYZ_USE_CONFIG_FILE
  • If a config file is not specified, try to find the flat config in the project root

(A flat config file has the name "eslint.config.[mc]?js")

Is there currently a default .eslintignore? Does the user's .eslintignore override it, or extend it?

@nvuillam
Copy link
Member

I knew the topic about eslint 9 would come at some point, I've not completely understood how required is the use of eslint.config.js as in beta the test cases still work :/

@ethanchristensen01
Copy link

ethanchristensen01 commented May 28, 2024

Frankly, I knew nothing about the flat config until just a few days ago. It seems like support for eslintrc will be dropped when v10 releases, so megalinter may eventually need to support two versions of eslint. 😕

Maybe the FlatCompat utility can help?

@ethanchristensen01
Copy link

ethanchristensen01 commented May 29, 2024

So, I have some good news.

Under the right circumstances, this configuration does currently work:

JAVASCRIPT_ES_CONFIG_FILE: ./eslint.config.mjs # or .js, .cjs
JAVASCRIPT_ES_COMMAND_REMOVE_ARGUMENTS: ["--no-eslintrc"] # Not valid option for eslint with flat config

Here are the requirements:

  1. There must be no .eslintignore in the root directory.

    • Having .eslintignore causes the Invalid option '--ignore-path' error
  2. ESLINT_FLAT_CONFIG (as an environment variable) must be true

    • If it's false, an error occurs which depends on the config. I was getting this:
      YAMLException: Cannot read config file: /tmp/lint/eslint.config.mjs
      Error: end of the stream or a document separator is expected
      
      This is because the old eslintrc parser is trying (and failing) to read the new config file.
    • Right now, I'm running megalinter through the npx tool. This is how I set the environment variable:
      npx mega-linter-runner -e "ENABLE_LINTERS=JAVASCRIPT_ES" -e "ESLINT_USE_FLAT_CONFIG=true"
      
      It would be nice to know if this can be set in .mega-linter.yml :)

This solution, although hacky, should work for supporting flat config in ESlint 8.

@nvuillam
Copy link
Member

nvuillam commented May 29, 2024

@ethanchristensen01 thanks for the analysis :)

What is your suggestion so we can by default use eslint.config.js by default according to eslint 9 and still avoid a crash for MegaLinter users who have eslint 8 config ? ( we can play with python to detect files at the root of the repo and use different arguments / env vars depending what we find ^^ )

@echoix
Copy link
Collaborator

echoix commented May 29, 2024

It's weird that you had to use the env var ESLINT_USE_FLAT_CONFIG and also have your eslint.config.js at the root of the repo. The docs specifically say that it is "or".

https://eslint.org/docs/latest/use/configure/migration-guide#start-using-flat-config-files

That might be something on our end, where if there isn't any valid config files (we need to add more files names), we add a default file that makes it so the repo has both files in the root of the repo.

@ethanchristensen01
Copy link

@echoix You might be right. Adding .eslintrc and .eslintignore might be causing that.

I noticed these errors at the bottom of my lint logs.

[Updated Sources Reporter] Unable to copy .eslintignore to /tmp/lint/megalinter-reports/updated_sources/.eslintignore ([Errno 2] No such file or directory: '.eslintignore')
[Updated Sources Reporter] Unable to copy MyProject/.eslintignore to /tmp/lint/megalinter-reports/updated_sources/MyProject/.eslintignore ([Errno 2] No such file or directory: 'MyProject/.eslintignore')
[Updated Sources Reporter] copied X fixed source files in folder /tmp/lint/megalinter-reports/updated_sources.

(last line added for context)

This might suggest there's a file cache of some kind, and it just happens to be working for me because it thinks I still have a .eslintignore in my project. That might prevent the default .eslintignore from getting copied in. (totally just a guess, haven't tested this theory yet)

@nvuillam Good question!

For now, I'm assuming we're going to add a configuration variable like JAVASCRIPT_ES_CONFIG_USE_FLAT_FILE.

I made a flowchart that should hopefully cover all cases!

flowchart TD;
    start(((Start)))
    q0["Has the current ESLint version abandoned eslintrc support?"]
    y0[["Use flat config"]]
    q1["Is the 'JAVASCRIPT_ES_USE_FLAT_CONFIG' configuration variable set?"]
    y1q1["Is that variable true?"]
    y1y1[["Use flat config"]]
    y1n1[["Use eslintrc config"]]
    q2["Is the environment variable 'ESLINT_USE_FLAT_CONFIG' set?"]
    q3["Is the configuration variable 'JAVASCRIPT_ES_CONFIG_FILE' set?"]
    y3q1["Is it eslint.config.{js, cjs, mjs}?"]
    y3y1[["Use flat config"]]
    y3n1q1["Are we using ESLint 9+?"]
    y3n1y1[["Use flat config"]]
    y3n1n1[["Use eslintrc config"]]
    q4["Is eslint.config.{js, cjs, mjs} in the project root?"]
    y4[["Use flat config"]]
    q5["Is .eslintrc* in the project root?"]
    y5[["Use eslintrc config"]]
    
    start --> q0
    q0 -->|Yes| y0
    q0 -.->|No| q1
    q1 -->|Yes| y1q1
    q1 -.->|No| q2
    y1q1 -->|Yes| y1y1
    y1q1 -.->|No| y1n1
    q2 -->|Yes| y1q1
    q2 -.->|No| q3
    q3 -->|Yes| y3q1
    q3 -.->|No| q4
    y3q1 -->|Yes| y3y1
    y3q1 -.->|No| y3n1q1
    y3n1q1 -->|Yes| y3n1y1
    y3n1q1 -.->|No| y3n1n1
    q4 -->|Yes| y4
    q4 -.->|No| q5
    q5 -->|Yes| y5
    q5 -.->|No| y3n1q1
Loading

@nvuillam
Copy link
Member

nvuillam commented Jun 2, 2024

@ethanchristensen01 amazing !
Let's implement that for new release :) ( after the one of today ^^ )

Copy link
Contributor

github-actions bot commented Jul 3, 2024

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jul 3, 2024
@echoix
Copy link
Collaborator

echoix commented Jul 3, 2024

I saw a discussion pass by on this in the super-linter repo.

@github-actions github-actions bot removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jul 4, 2024
@bdovaz
Copy link
Collaborator

bdovaz commented Jul 22, 2024

I just encountered this problem 😅

@nvuillam
Copy link
Member

@ethanchristensen01 gave us the specs... :D

If a good samaritan wanna take it, i'd be glad to check the PR, otherwise i'll try to find a way before the next major release ^^

Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Aug 22, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 5, 2024
@theetrain
Copy link
Author

An issue that was triaged as a bug shouldn't get marked as stale and then closed.

@nvuillam nvuillam reopened this Sep 8, 2024
@nvuillam nvuillam removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Sep 8, 2024
@nvuillam
Copy link
Member

nvuillam commented Sep 8, 2024

@theetrain indeed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants