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

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

Closed
yp05327 opened this issue Jun 19, 2024 · 19 comments
Closed

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

yp05327 opened this issue Jun 19, 2024 · 19 comments
Labels
info-needed Issue requires more information from poster

Comments

@yp05327
Copy link

yp05327 commented Jun 19, 2024

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 yp05327 changed the title After upgrade to version 3.x, lints json file as js/ts file After upgrade to version 3.x, json files are broken Jun 19, 2024
@dbaeumer
Copy link
Member

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

@dbaeumer
Copy link
Member

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

@dbaeumer dbaeumer added the info-needed Issue requires more information from poster label Jun 19, 2024
@yp05327
Copy link
Author

yp05327 commented Jun 20, 2024

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

@yp05327
Copy link
Author

yp05327 commented Jun 20, 2024

@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

@dbaeumer
Copy link
Member

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.

@yp05327
Copy link
Author

yp05327 commented Jun 23, 2024

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. 🤔

@dbaeumer
Copy link
Member

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

@yp05327
Copy link
Author

yp05327 commented Jun 25, 2024

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

@dbaeumer
Copy link
Member

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

@yp05327
Copy link
Author

yp05327 commented Jun 25, 2024

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.

@yp05327
Copy link
Author

yp05327 commented Jun 25, 2024

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.

@yp05327
Copy link
Author

yp05327 commented Jun 25, 2024

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.

@yp05327
Copy link
Author

yp05327 commented Jun 26, 2024

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.

@dbaeumer
Copy link
Member

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.

@dbaeumer
Copy link
Member

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.

@yp05327
Copy link
Author

yp05327 commented Jun 26, 2024

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.

@yp05327
Copy link
Author

yp05327 commented Jun 26, 2024

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.

@dbaeumer
Copy link
Member

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.

@dbaeumer
Copy link
Member

I opened #1892 for the validate setting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

2 participants