AtomLinter/linter-eslint

.eslintrc.cjs not checked with Disabling option

brettz9 opened this issue ยท 11 comments

Issue Type

Bug if interpreted as how probably should work.

Issue Description

When "Disable when no ESLint config is found (in package.json or .eslintrc)" is checked, this can be overly aggressive if one's .eslintrc is a .eslintrc.cjs file.

Thanks!

Bug Checklist

  • Restart Atom
  • Verify the eslint CLI gives the proper result, while linter-eslint does not
  • Paste the output of the Linter Eslint: Debug command from the Command Palette below - N/A
Linter Eslint: Debug output here

I think this was fixed in #1068.

Comment if it is not fixed yet.

With the item checked, it still disables checking (which it shouldn't).

But there is another problem (which I'm not sure was present before): With disable unchecked, it says "Error when running ESLint: No ESLint configuration found..." despite my having .eslintrc.cjs.

For the record, in the recent version of linter-eslint, the config file is found by Eslint's CLIEngine. So, the issue is either in Eslint itself or the code for this atom config.

export function getConfigForFile(eslint, filePath) {

My apologies, as far as the item being unchecked, I hadn't installed on the repository I was temporarily testing just now. (Still, before adding dependencies, linter-eslint gave what seems like the wrong error, but with the dependencies installed, it now correctly reports ESLint errors.)

But when checked, it indeed fails to report anything as it should.

You don't need to install the repository. Just install the official version:

apm uninstall linter-eslint
apm install linter-eslint

This is the same as installing it from inside Atom

Sorry, what I mean was that on my own repository/directory that I was linting, I hadn't installed the eslint dependency yet (When I first reported the issue I had, but when checking just now, I had been working on a new repository.)

So here are the issues that I am still seeing:

  1. When eslint is not installed, but there is an .eslintrc.cjs in my directory, I get the error, "Error when running ESLint: No ESLint configuration found...". I'd expect an error like "No eslint binary found". Not a big issue, especially now that I know this, but something you might want to look into.
  2. The original reported issue that when "Disable when no ESLint config is found (in package.json or .eslintrc)" is checked, an .eslintrc.cjs (instead of other type) will disable ESLint checking. (But linter-eslint works as expected in reporting linting erors, even with .eslintrc.cjs, when the checkbox is unchecked.)

I think we should provide a default Eslint config instead of throwing errors in every project. The current default experience is not well selected (especially for beginners).

Whatever defaults you might provide, I would request keeping the opportunity (e.g., by option) to get an error (of some kind) because it helps one realize that one may need to add linting (and doesn't give the false comfort that strict linting is supposedly being applied when it isn't).

(Despite discovering this issue, I normally like not to check the disable option.)

Whatever defaults you might provide, I would request keeping the opportunity (e.g., by option) to get an error (of some kind) because it helps one realize that one may need to add linting (and doesn't give the false comfort that strict linting is supposedly being applied when it isn't).

(Despite discovering this issue, I normally like not to check the disable option.)

Yes. I'll keep the option

When eslint is not installed, but there is an .eslintrc.cjs in my directory, I get the error

That may be because the bundled eslint is v4 and probably doesn't recognize .cjs as a valid config extension.

We are in the process of updating the bundled version in #1360

๐ŸŽ‰ This issue has been resolved in version 9.0.0 ๐ŸŽ‰

The release is available on:

Your semantic-release bot ๐Ÿ“ฆ๐Ÿš€