sveltejs / kit

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

ESLint misses broken imports, incorrectly reports $lib imports as broken or reports nothing at all #1560

Open KonradHoeffner opened 3 years ago

KonradHoeffner commented 3 years ago

Describe the bug I had the problem that my ESLint configuration throws errors on $lib/... imports so I created a new SvelteKit project to find out how that should be handled. However there I have the opposite problem that ESLint import checking is not working at all, as it doesn't even warn me when an import does not exist.

Logs and To Reproduce There is no output from eslint even though 'thisdoesnot/exist.js' is not there and fails in the npm run dev step:

npm init svelte@next sveltetest
cd sveltetest
npm install
echo "<script>import doesnotexist from 'thisdoesnot/exist.js'</script>" >> src/routes/index.svelte
$ npm run lint      

> ~TODO~@0.0.1 lint
> prettier --check --plugin-search-dir=. . && eslint --ignore-path .gitignore .

Checking formatting...
[warn] package-lock.json
[warn] package.json
[warn] src/routes/index.svelte
[warn] Code style issues found in the above file(s). Forgot to run Prettier?
sveltetest$ eslint --ignore-path .gitignore .

/tmp/sveltetest/src/routes/index.svelte
  3:16  warning  'doesnotexist' is defined but never used  @typescript-eslint/no-unused-vars

✖ 1 problem (0 errors, 1 warning)

npm run dev -- --open
> ~TODO~@0.0.1 dev
> svelte-kit dev

  SvelteKit v1.0.0-next.109

  local:   http://localhost:3000
  network: not exposed

  Use --host to expose server to other devices on this network

Failed to resolve import "thisdoesnot/exist.js" from "src/routes/index.svelte". Does the file exist?

Expected behavior

  1. Correct imports should not be shown as errors, even if they contain $lib.
  2. Incorrect imports should cause errors.
  3. ESLint output should always be visible when calling npm run lint.

Information about your SvelteKit Installation:

Diagnostics - The output of `npx envinfo --system --npmPackages svelte,@sveltejs/kit,vite --binaries --browsers` ``` System: OS: Linux 5.12 Arch Linux CPU: (8) x64 Intel(R) Core(TM) i7-9700 CPU @ 3.00GHz Memory: 7.21 GB / 15.42 GB Container: Yes Shell: 5.1.8 - /bin/bash Binaries: Node: 16.2.0 - /usr/bin/node npm: 7.14.0 - /usr/bin/npm Browsers: Chromium: 90.0.4430.212 npmPackages: @sveltejs/kit: next => 1.0.0-next.109 svelte: ^3.34.0 => 3.38.2 ``` - Your browser: Firefox 89 - Your adapter (e.g. Node, static, Vercel, Begin, etc...): The default one from npm init svelte@next, maybe static?

Severity It is not blocking but annoying.

Additional context I think there are three issues relating to ESLint:

  1. When using the SvelteKit default ESLint configuration, ESLint does not notice broken imports at all.
  2. When using another ESLint configuration, such as the one from the SvelteKit repository, '$lib' errors that are correct cause errors by ESLint.
  3. Even if ESLint notices any errors or warnings, npm run lint does not output them. I assume that prettier exits with an error code and stops the second part of the && from being executed.
dummdidumm commented 3 years ago

Point 3 is "works as designed". Your assumption is correct, and if you want to change that, format the files first or switch the order of invocation.

justingolden21 commented 2 years ago

Is there any fix for this? All of my files are underlined in red with ESLint, and I'd rather not disable the entire rule checking unresolved imports. Would be awesome. Thanks 😄

iva2k commented 2 years ago

I added a bit of stuff to the setup to resolve the issue like this:

  1. npm i -D eslint-plugin-import eslint-import-resolver-typescript

  2. Add few lines to .eslintrc.cjs:

    {
    extends: [
    'eslint:recommended',
    'plugin:@typescript-eslint/recommended',
    +    'plugin:import/recommended',
    'prettier'
    ],
    plugins: [
    ...
    +    'import'
    ],
    settings: {
    +    'import/resolver': {
    +      typescript: {}
    +    },
    ...
    },
    parserOptions: {
    +    project: ['./tsconfig.json', './tsconfig.lint.json'],
    +    tsconfigRootDir: './',
    ...
    }
    }
  3. Create file tsconfig.lint.json with:

    {
    "extends": "./tsconfig.json",
    "include": ["./playwright.config.ts", "./svelte.config.js", "./tests/**/*.ts"]
    }
  4. Add few lines to tsconfig.json:

    "compilerOptions": {
    ...
    +    "paths": {
    +      "$app": ["./.svelte-kit/runtime/app"],
    +      "$app/*": ["./.svelte-kit/runtime/app/*"],
    +      "$lib": ["./src/lib"],
    +      "$lib/*": ["./src/lib/*"]
    +    }
    }
    }

These changes fix the issue for me. I think those deserve to be added to svelte-kit templates.

stalkerg commented 2 years ago
  1. Because now all load functions in separate js files it's very annoying!!!
  2. @iva2k not working for me because I am not using TS.

Does somebody have a solution?

stalkerg commented 2 years ago

Okay, I solve it for ESLint by eslint-import-resolver-custom-alias plugin. This is my config:

    "settings": {
        "import/resolver": {
            "eslint-import-resolver-custom-alias": {
                "alias": {
                    "$lib":"./src/lib",
                    "$app":"./.svelte-kit/runtime/app",
                    "@sveltejs":"./.svelte-kit/dev"
                },
                "extensions": [".js"]
            },
        }
    },

I think we should have this in the default template.

stalkerg commented 1 year ago

@ota-meshi seems like eslint-plugin-svelte incompatible with eslint-import-resolver-custom-alias and I see next errors:

npx eslint src/lib/Image.svelte 

Error while parsing /home/stalkerg/projects/mjv-art.org/spa/src/lib/i18n.js
Line 15, column 0: Unexpected token
`parseForESLint` from parser `/home/stalkerg/projects/mjv-art.org/spa/node_modules/svelte-eslint-parser/lib/index.js` is invalid and will just be ignored

Error while parsing /home/stalkerg/projects/mjv-art.org/spa/src/lib/helpers.js
Line 3, column 14: Expected }
`parseForESLint` from parser `/home/stalkerg/projects/mjv-art.org/spa/node_modules/svelte-eslint-parser/lib/index.js` is invalid and will just be ignored

/home/stalkerg/projects/mjv-art.org/spa/src/lib/Image.svelte
  48:1  error  This line has a length of 101. Maximum allowed is 100  max-len

Basically, it's happened if I import such JS from .svelte

stalkerg commented 1 year ago

It seems like it will be good to write a new eslint-import-resolver- to support paths from jsconfig.json . UPDATE it's exist https://www.npmjs.com/package/eslint-import-resolver-jsconfig ! I will check. UPDATE2 it's not working as expected, but closer

stalkerg commented 1 year ago

Okey, seems like 'eslint-config-airbnb-base' for some reason breaks the default resolver.

stalkerg commented 1 year ago

Because new eslint-plugin-svelte using a ts parser inside, two rules from airbnb break it. It means we should disable it and add a new aliases, my new config works fine and looks like:

  "rules": {
    "import/named": "off",
    "import/no-cycle": "off"
  },
  "settings": {
    "import/resolver": {
      "eslint-import-resolver-custom-alias": {
        "alias": {
          "$lib": "src/lib",
          "$app": "node_modules/@sveltejs/kit/src/runtime/app",
          "@sveltejs/kit": "node_modules/@sveltejs/kit/src/exports/index.js"
        },
        "extensions": [
          ".js"
        ]
      }
    }
  }

it's of course it's addition to the default config from svelte-create and aribnb preset.

@dummdidumm can we add it in svelte-create? Also, I created a feature request here https://github.com/sveltejs/eslint-plugin-svelte/issues/453

dummdidumm commented 1 year ago

Are these rules (import/..) enabled in the svelte-create setup? If yes, then it makes sense to me to add the rules adjustments, else not.

stalkerg commented 1 year ago

Are these rules (import/..) enabled in the svelte-create setup?

@dummdidumm As I can see, no, but it's not fair because the typescript setup does such a test by TS tools. Our TS default setup validates imports, but our JS setup, even with ESLint, does not include the import plugin by default.

stalkerg commented 1 year ago

Okey, I should say thanks @ota-meshi he find a better setup:

  "import/parsers": {
      "svelte-eslint-parser": [".svelte"],
      "espree": [".js"]
  },
  "settings": {
    "import/resolver": {
      "eslint-import-resolver-custom-alias": {
        "alias": {
          "$lib": "src/lib",
          "$app": "node_modules/@sveltejs/kit/src/runtime/app",
          "@sveltejs/kit": "node_modules/@sveltejs/kit/src/exports/index.js"
        },
        "extensions": [
          ".js"
        ]
      }
    }
  }

basically, we need an extra parser if we want to use import plugin.