megalinter icon indicating copy to clipboard operation
megalinter copied to clipboard

Unable to use ESLint flat config for linting JavaScript

Open theetrain opened this issue 1 year ago • 33 comments

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 avatar May 22 '24 02:05 theetrain

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 avatar May 27 '24 20:05 ethanchristensen01

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.

ethanchristensen01 avatar May 27 '24 21:05 ethanchristensen01

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

echoix avatar May 27 '24 21:05 echoix

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.

ethanchristensen01 avatar May 27 '24 21:05 ethanchristensen01

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):

https://github.com/oxsecurity/megalinter/blob/c0409fa8f071903bd51621800f7aad68869687ec/megalinter/descriptors/javascript.megalinter-descriptor.yml#L31-L53

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?

echoix avatar May 27 '24 21:05 echoix

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?

ethanchristensen01 avatar May 28 '24 20:05 ethanchristensen01

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 :/

nvuillam avatar May 28 '24 21:05 nvuillam

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 avatar May 28 '24 22:05 ethanchristensen01

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.

ethanchristensen01 avatar May 29 '24 20:05 ethanchristensen01

@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 ^^ )

nvuillam avatar May 29 '24 20:05 nvuillam

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.

echoix avatar May 29 '24 21:05 echoix

@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

ethanchristensen01 avatar May 29 '24 23:05 ethanchristensen01

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

nvuillam avatar Jun 02 '24 09:06 nvuillam

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[bot] avatar Jul 03 '24 00:07 github-actions[bot]

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

echoix avatar Jul 03 '24 04:07 echoix

I just encountered this problem 😅

bdovaz avatar Jul 22 '24 13:07 bdovaz

@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 ^^

nvuillam avatar Jul 22 '24 20:07 nvuillam

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[bot] avatar Aug 22 '24 00:08 github-actions[bot]

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

theetrain avatar Sep 05 '24 01:09 theetrain

@theetrain indeed :)

nvuillam avatar Sep 08 '24 22:09 nvuillam

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[bot] avatar Oct 09 '24 01:10 github-actions[bot]

Just encountered this as well. Following the @ethanchristensen01 workaround, this is what I've done in the .mega-linter.yml. Seems like a working solution until a fix is pushed and a new version of mega-linter is released.

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

PRE_COMMANDS:
  # temporary workaround for supporting flat config in eslint
  # from https://github.com/oxsecurity/megalinter/issues/3570#issuecomment-2138193684
  # first ensure .eslintignore is not present
  - command: rm -f .eslintignore
    cwd: workspace
  # then add ESLINT_FLAT_CONFIG=true to ENV
  - command: export ESLINT_FLAT_CONFIG="true"
    # Will collect the values of output variables and update MegaLinter own ENV context
    # See https://github.com/oxsecurity/megalinter?tab=readme-ov-file#pre-commands
    output_variables: ["ESLINT_FLAT_CONFIG"]

serpro69 avatar Oct 11 '24 18:10 serpro69

  # first ensure .eslintignore is not present
  - command: rm -f .eslintignore
    cwd: workspace

@serpro69 Removing the .eslintignore file by force might be an unexpected side effect.

  - command: export ESLINT_FLAT_CONFIG="true"
    # Will collect the values of output variables and update MegaLinter own ENV context
    # See https://github.com/oxsecurity/megalinter?tab=readme-ov-file#pre-commands
    output_variables: ["ESLINT_FLAT_CONFIG"]

Thanks for figuring out how to configure this in the megalinter file!

ethanchristensen01 avatar Oct 11 '24 18:10 ethanchristensen01

Removing the .eslintignore file by force might be an unexpected side effect.

Yeah, that was just a dirty hackaround. After reading a little bit about the flat config, I can see that our repo wasn't properly configured for using it in the first place, and we had both .eslintignore and eslint.config.mjs. So I just made sure the ignore file is removed before running the linter in a github action (we're not using the mega-linter locally, so it shouldn't produce any bad side-effects that I can think of) to see if this will work. But I just read the migration guide, particularly the ignoring files section, and you need to ignore files differently with a flat config anyways. So I've removed the ignore file from code altogether. And if your project is correctly configured for using flat config - this command shouldn't be needed because you shouldn't be having .eslintignore there in the first place.

Thanks to you too for providing a working solution for this issue! :fist_right: :fist_left:

serpro69 avatar Oct 11 '24 19:10 serpro69

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[bot] avatar Nov 11 '24 01:11 github-actions[bot]

Maybe consider removing the stale GitHub action and opt for manual triaging and roadmapping instead.

Or set days_before_close to -1.

https://github.com/oxsecurity/megalinter/blob/2e057ee330278ced6642908470ddb4b19488e2b5/.github/workflows/stale.yml#L37

Reference: https://github.com/actions/stale?tab=readme-ov-file#days-before-close

theetrain avatar Nov 25 '24 01:11 theetrain

We have a label for ignoring it permanently on a PR or issue, but it does its job of pinging people back when there is lost interest

echoix avatar Nov 25 '24 01:11 echoix

All it needs is someone to take the action to apply @ethanchristensen01 's analysis and solution :)

It will probably end being me... but it's probably a full day of work and it's hard to find it ^^

nvuillam avatar Nov 26 '24 19:11 nvuillam

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[bot] avatar Dec 27 '24 01:12 github-actions[bot]

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[bot] avatar Feb 19 '25 01:02 github-actions[bot]