sveltejs / kit

web development, streamlined
https://kit.svelte.dev
MIT License
18.39k stars 1.88k forks source link

Switch default eslint plugin #6847

Closed Jojoshua closed 1 year ago

Jojoshua commented 1 year ago

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 https://github.com/sveltejs/eslint-plugin-svelte3/issues/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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

Creating the migration guide is my task but still I didn't start. https://github.com/ota-meshi/eslint-plugin-svelte/issues/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 commented 1 year ago

@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"
            }
        }
    ]
}
ota-meshi commented 1 year ago

@ZetiMente I know it's off topic for this thread, but I think I can probably help you. Can you share a repository that reproduces your issue in the plugin's repository?

kevin-in-code commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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