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

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

Closed
KonradHoeffner opened this issue May 26, 2021 · 13 comments
Milestone

Comments

@KonradHoeffner
Copy link

KonradHoeffner commented May 26, 2021

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.
@KonradHoeffner KonradHoeffner changed the title ESLint misses broken imports ESLint misses broken imports, incorrectly reports $lib imports as broken or reports nothing at all May 26, 2021
@dummdidumm
Copy link
Member

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
Copy link

justingolden21 commented May 13, 2022

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
Copy link

iva2k commented May 29, 2022

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: './',
    ...
  }
}
  1. Create file tsconfig.lint.json with:
{
  "extends": "./tsconfig.json",
  "include": ["./playwright.config.ts", "./svelte.config.js", "./tests/**/*.ts"]
}
  1. 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
Copy link
Contributor

  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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Contributor

stalkerg commented Apr 27, 2023

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
Copy link
Contributor

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

@stalkerg
Copy link
Contributor

stalkerg commented Apr 28, 2023

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 sveltejs/eslint-plugin-svelte#453

@dummdidumm
Copy link
Member

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
Copy link
Contributor

stalkerg commented Apr 28, 2023

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
Copy link
Contributor

stalkerg commented May 1, 2023

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.

kyoshino added a commit to sveltia/sveltia-ui that referenced this issue Jun 12, 2023
@eltigerchino
Copy link
Member

Closing this because I can't seem to reproduce the issue with the latest SvelteKit. $lib aliases are correctly resolved by eslint without the custom alias resolver plugin.

@eltigerchino eltigerchino closed this as not planned Won't fix, can't repro, duplicate, stale Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants