microsoft/vscode-eslint

After upgrade to version 3.x, json files are broken

yp05327 opened this issue ยท 19 comments

It works well in 2.4.4, but after I upgrade to the latest 3.x.x, it seems that it will lint all .json as js/ts file
In 2.4.4:
image
In 3.0.8 and 3.0.10:
image

@yp05327 can you provide me with a GitHub repository I can clone that demos this. Problems like this are sensitive to versions (elint, plugins, ...) and I want to ensure we are using the same setup.

I tried to reproduce this but failed. A GitHub repository to clone is highly appreciated.

I'm using a payed Vue template, so I can't push all of them. I will try to provide a demo later.

@dbaeumer
I have done. You can clone this repo:
https://github.com/yp05327/eslinterror

Reproduce:

  • Open it in dev container
  • Run pnpm install in terminal
  • Reload window (press F1 and select Developer: Reload Window)

Then you can see this issue:
image

I notice that the following two things effecting this issue:

  • You should run pnpm install, and reload the window
  • You should not delete .eslintrc.cjs

I have tested it in my local PC in a clean space again, so I'm sure you can reproduce it.

I didn't modify .eslintrc.cjs, but I removed several custom commands in packages.json which will cause error during the installation as not all files are uploaded, and this is not related to this issue

I was able to reproduce, however the same errors seem to be reported on the command line as well (see screen shot).

Image

So there seems to be a more general problem with the setup since this happens independent of the ESLint extension.

So this issue comes from eslint self but not the extension?
One thing I couldn't understand, why after I downgrade the extension, it worked.
And eslint's version is hard coding (in package.json), so eslint version and configs are all the same.
The only difference between these are the extension's version. This seems strange. ๐Ÿค”

It sounds strange. May be the extension doesn't work when downgraded :-). Does it work in general?

image
These two conditions are required, or you will not see the errors.

I followed these steps and still see the errors both in the editor AND when running eslint directly in the terminal. Since these errors are present in the terminal it is IMO correct that they are shown in the extension as well.

Image

If I downgrade the extension, you will see nothing happened, but eslint still reports errors.
image
Maybe it is better to report this issue to eslint not here

So I guess, maybe eslint extension will ignore these no-related errors in 2.x, but will not do that in 3.x.

I notice that the missing rules valid-appcardcode-code-prop are user defined rules.
I uploaded missing files in yp05327/eslinterror@59b10bb

valid-appcardcode-code-prop is defined in eslint-internal-rules, and imported by .vscode/settings.json
so if you just run eslint in command line, it should display errors, as this rule is not imported.

So the correct command is ./node_modules/.bin/eslint --rulesdir eslint-internal-rules/ test.json :
image

There are still some errors but rule not found error disappeared. So I think, at least, the settings options eslint.options.rulePaths is broken in 3.x.

I found the solution:
the lint command defined in packages.json is

"lint": "eslint . -c .eslintrc.cjs --fix --rulesdir eslint-internal-rules/ --ext .ts,.js,.cjs,.vue,.tsx,.jsx"

Then I added these options in .vscode/settings.json
image

And then there's no errors any more.
image

--fix is not related, but config is related I think.
So maybe the root reason is that .eslintrc.cjs can not be automatically loaded by the extension by default.

I confirmed that this is partly caused by #1787
In this PR, json files are added. So in 2.x, json files will not be triggered, but in 3.x, it will.

And then because of the bad logic here:
image
If user didn't even enable the plugins in .eslintrc.cjs, extension will still validate these files.
In my case, this project don't use json lint, but it will be detected by the extension.

Maybe a fix can be checking whether user enabled the plugin in eslint config file, then mark settings.validate to on or off.

Happy to hear you could get it to work

Actually the code checks if the plugin is enabled by loading the configuration of the file. In your setup the following plug-ins are available according the the eslint npm package.

Image

I don't check config files since configuration can come from many many place. So I check what ESLint tells me the configuration looks like.

I think the problem is that .eslintrc.cjs files are only honored when running inside a module package. This is why you need to pass it on the command line since it is not the case in the setup of the example repository.

I double checked the code and IMO it does the right thing.

I will close the issue. Please ping if you think somethings should be done.

In my demo, you can see that jsonc is in eslint.plugins list, but I don't have it in packages.json.
So, I think eslint.plugins means eslint provides these plugins, but not means user has installed and want to use it.
I don't think it does the right thing.

I clean the node_modules, and runned pnpm install again. It will install eslint-plugin-jsonc for you, even it is not listed in packages.json, that's why jsonc is in eslint.plugins list.
image

Maybe it is required by some packages, so installed automatically.
But I don't think it is right to get plugins from the installed plugins list, as in some cases it is not expected.

So once I installl other packages, eslint-plugin-jsonc will also be installed, and then, if I don't want to lint json files, how to avoid it? Although I have found one solution, but is it the best way to do that?
No. Adding config=".eslintrc.cjs" seems stupid, just like I need to tell npm the packages config file is "package.json", or it will give you errors.

But ESLint does the same thing on the command line. Since I use the same code (and I don't want to do my own parsing of config files) I get the same result.

What I can think of is to enforce the validate setting. If a user has defined it and the file type is not in the list it will not validate the file. This would be an equal behavior as specifying things on the command line.

I opened #1892 for the validate setting