Skip to content

Switch default eslint plugin #6847

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

Closed
Jojoshua opened this issue Sep 15, 2022 · 9 comments · Fixed by #9749
Closed

Switch default eslint plugin #6847

Jojoshua opened this issue Sep 15, 2022 · 9 comments · Fixed by #9749
Milestone

Comments

@Jojoshua
Copy link

Describe the problem

If setting up a new sveltekit project via npm create svelte@latest and you request eslint support, you will by default get setup with https://github.com/sveltejs/eslint-plugin-svelte3

The issue is that eslint-plugin-svelte3 is not a correct parser and there have been a lot of issues with its usage. eslint-plugin-svelte just works better and is staying more maintained.

References
sveltejs/eslint-plugin-svelte3#184
https://github.com/ota-meshi/eslint-plugin-svelte#-why

Describe the proposed solution

Before going v1 change the default use the eslint plugin https://github.com/ota-meshi/eslint-plugin-svelte

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

@Conduitry
Copy link
Member

I don't think we necessarily want to hold up v1 for this. That ESLint plugin will need to be more thoroughly vetted and brought under the sveltejs org before we include it in templates. Luckily, we can switch this out as a breaking change in create-svelte if necessary, without it being a breaking change in SvelteKit itself.

@ZetiMente
Copy link

ZetiMente commented Sep 19, 2022

@Jojoshua is there documentation for converting from eslint-plug-svelte3 to eslint-plug-svelte ?

I migrated but getting a bunch of these errors for all my config files:

 0:0  error  Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
 The file does not match your project config: windi.config.ts.
The file must be included in at least one of the projects provided
 

And for this code it returns an error:

const like = async () => {};
<a href="#" on:click={like}

Says 'like' is not defined. While the eslint-plugin-svelte3 doesn't return an error.

@Jojoshua
Copy link
Author

@ZetiMente I followed https://ota-meshi.github.io/eslint-plugin-svelte/user-guide/

I would ask @baseballyama or @ota-meshi to chime in if they know of a better article.

@baseballyama
Copy link
Member

Creating the migration guide is my task but still I didn't start.
sveltejs/eslint-plugin-svelte#252
So for now, I think there is no better article for it.

@ZetiMente

Did you finish switching the plugin after following https://ota-meshi.github.io/eslint-plugin-svelte/user-guide/ ?
If there is difficult part, I would appreciate it if you could let me know.
I will use it as reference for writing the migration guide.

@ZetiMente
Copy link

ZetiMente commented Sep 19, 2022

@baseballyama Do I need to add *.config.ts & *.config.js files to the eslint ignore or did I mess up something? Seems to be the only eslint migration issue.

Screen Shot 2022-09-19 at 2 45 12 PM

{


	"root": true,
	"parser": "@typescript-eslint/parser",
	"plugins": ["@typescript-eslint"],
	"parserOptions": {
		"ecmaVersion": 2020,
		"sourceType": "module",
		"project": "tsconfig.json",
		"extraFileExtensions": [".svelte"] // This is a required setting in `@typescript-eslint/parser` v4.24.0.
	},
	"ignorePatterns": [
		"*.cjs",
		"*.svg",
		"*.webp",
		"*.sql",
		"*.pgsql",
		"*.test.js",
		"*.sh",
		"static/",
		"images/",
		"prisma/",
		"supabase/",
		"README.md",
		"pnpm-lock.yaml",
		"netlify.toml",
		"app.html",
		"error.html",
		"app.scss"
	],
	"env": {
		"es2020": true,
		"node": true, // for config files
		"browser": true
	},
	"extends": [
		"eslint:recommended",
		"plugin:@typescript-eslint/recommended",
		"plugin:security/recommended",
		"plugin:svelte/recommended"
		// "plugin:@typescript-eslint/recommended-requiring-type-checking"
	],
	"overrides": [
		{
			"files": ["*.svelte"],
			"parser": "svelte-eslint-parser",
			// Parse the `<script>` in `.svelte` as TypeScript by adding the following configuration.
			"parserOptions": {
				"parser": "@typescript-eslint/parser"
			}
		}
	]
}

@Rich-Harris Rich-Harris added this to the whenever milestone Sep 19, 2022
@benmccann benmccann changed the title Switch default eslint plugin before v1 Switch default eslint plugin Sep 20, 2022
@ota-meshi

This comment was marked as off-topic.

@kevin-in-code
Copy link

@baseballyama I have created an example of adopting eslint-plugin-svelte using the sveltekit demo app. There is a slight trap which @ZetiMente reported. I think the most robust solution is to use a tsconfig.eslint.json file extending the project's tsconfig.json, as shown in the example.

@baseballyama
Copy link
Member

@kevin-a-naude

Thank you for the REPL. I think some people also will face the issue, so I think we should add some statements to the doc. (Also previously I faced the same issue😅)
But this is just curious but why didn't you add "include": ["**/*.js", "**/*.ts", "**/*.svelte"] to tsconfig.json ?

@kevin-in-code
Copy link

@baseballyama

But this is just curious but why didn't you add "include": ["**/*.js", "**/*.ts", "**/*.svelte"] to tsconfig.json ?

That is a viable solution for the demo app. I hesitate to suggest it as a general recommendation as it is a bit of a broad brush. There may be tools or build processes in the wild that read tsconfig.json for some purpose, and these could produce different output if the change is made there.

There are two other alternatives worth considering. One of these is to use files: ['playwright.config.ts', 'svelte.config.js', 'vite.config.ts'] in either tsconfig.eslint.json or tsconfig.json rather than include. This would be a narrower change because we wouldn't be completely overriding the project's include option; we probably shouldn't make assumptions about what might be in there in general.

The documentation for tsconfig.json extends warns about file, include, and exclude overriding, and I think we should probably give some thought to that as well.

It’s worth noting that files, include and exclude from the inheriting config file overwrite those from the base config file, and that circularity between configuration files is not allowed.

The other approach that works in the put *.config.ts and *.config.js into .eslintignore. This is a neat solution with one major drawback: the linter will no longer point out issues in there config files.

I am leaning towards an explicit files entries in tsconfig.json as the best general approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants