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

Not working with eslint-plugin-import and yarn 3 #1494

Closed
brunomacf opened this issue Jul 3, 2022 · 23 comments
Closed

Not working with eslint-plugin-import and yarn 3 #1494

brunomacf opened this issue Jul 3, 2022 · 23 comments
Labels
info-needed Issue requires more information from poster

Comments

@brunomacf
Copy link

I'm not sure if this is an issue with eslint-plugin-import or vscode-eslint itself but i created this repo https://github.com/brunomacf/vscode-eslint-yarn-berry reproducing the issue. If i open src/index.ts in vscode i don't see any lint errors (i do see the correct errors when i totally remove the eslint-plugin-import settings from .eslintrc.js config file). But if i run lint manually through yarn lint i see the lint errors just fine (bellow is a screenshot showing on the left the result of yarn lint on the left and on the right vscode showing no lint errors). Any ideas? PS.: I created a similar issue in eslint-plugin-import here

Captura de Tela 2022-07-03 às 17 38 08

@dbaeumer
Copy link
Member

dbaeumer commented Jul 4, 2022

@brunomacf according to the yarn documentation you need to run yarn dlx @yarnpkg/sdks vscode to make yarn pnp work with VS Code (this is not only a general VS Code problem it is that yarn pnp changes the way how node load things).

VS Code then runs eslint from .yarn/sdks/eslint/lib/api.js. If I do that on the command line using node ./.yarn/sdks/eslint/lib/api.js ./src/index.ts I get actually no errors either. So this seems to be a setup problem with either your workspace or yarn dlx @yarnpkg/sdks vscode

I have to admit that I am not a super expert with yarn pnp but in general yarn's sdks for editors worked so far for me.

I am absolutely open for tip on how to improve this for VS Code.

@dbaeumer dbaeumer added the info-needed Issue requires more information from poster label Jul 4, 2022
@brunomacf
Copy link
Author

@dbaeumer I don't understand what is going on tbh. If i remove all eslint-plugin-import stuff from .eslintrc.js file and add a console.log call in src/index.js i see vscode-eslint complaining about it (screenshot bellow). If eslint-plugin-import stuff is in eslint.js i can see eslint itself complaining about the console.log when running eslint in terminal via yarn eslint . --ext .ts (second screenshot bellow) although running eslint via .yarn/sdks/eslint/lib/api.js indeed does not show anything. But what intrigues me is the fact that vscode-eslint is working when i remove eslint-plugin-import stuff.

Captura de Tela 2022-07-04 às 08 30 32
Captura de Tela 2022-07-04 às 08 31 10

@ljharb
Copy link

ljharb commented Jul 4, 2022

It’s because the import plugin relies on the proper way to resolve things - ie, what node ships - and since pnp breaks this, pnp and the import plugin can’t be used together unless pnp’s workarounds are employed.

@brunomacf
Copy link
Author

@ljharb then why eslint-plugin-import work just fine when i run eslint from command line with yarn eslint . --ext .ts? In the screenshot bellow you can see it complains about import/order but in vscode there is no error at all:

Captura de Tela 2022-07-04 às 14 25 30

@ljharb
Copy link

ljharb commented Jul 4, 2022

Because then you’re invoking it with yarn, so yarn is able to hack the runtime to make it work - the editor doesn’t invoke eslint with yarn, it invokes it directly. If you ran npx eslint I’d expect it to break in the same way.

@dbaeumer
Copy link
Member

dbaeumer commented Jul 5, 2022

@ljharb thanks for helping me out here.

@brunomacf one thing you can do is to setup a task that runs eslint through yarn. This would not give you the live feedback but would benefit from yarn setting up the runtime correctly.

Since there is little the extension can do to fix this are you OK to close the issue? I would actually recommend to reopen the issue with the eslint-plugin-import pointing out that if you call eslint from the yarn sdk folder the plugin doesn't work correctly.

@ljharb
Copy link

ljharb commented Jul 5, 2022

I think this remains an issue with yarn pnp, tbh, but happy to discuss it on that issue as well.

@JounQin
Copy link

JounQin commented Jul 6, 2022

@brunomacf
Copy link
Author

@JounQin i guess it's release yet right? I just upgraded eslint-import-resolver-typescript to 3.2.4 in https://github.com/brunomacf/vscode-eslint-yarn-berry and i still cant see vscode-eslint complaining about incorrect import order:

Captura de Tela 2022-07-06 às 09 06 30

As you can see vscode-eslint doesn't even complain about the console.log even though i have set no-console rule. If i remove all stuff related to eslint-plugin-import from .eslintrc.js file then i start seeing vscode-eslint complaining about console.log:

Captura de Tela 2022-07-06 às 09 08 06

@JounQin
Copy link

JounQin commented Jul 6, 2022

@brunomacf Oh, sorry, so the resolving seems working but vscode-eslint and eslint-plugin-import are not compatible in this case, right?

@JounQin
Copy link

JounQin commented Jul 6, 2022

@brunomacf

yarn v3 requires yarn dlx @yarnpkg/sdks vscode, I think it may have to patch eslint-plugin-import like they do for eslint and typescript in this case?

image

Maybe you should post an issue to https://github.com/yarnpkg/berry/blob/HEAD/packages/yarnpkg-sdks instead?

It's not vscode-eslint or eslint-plugin-import's responsibility to support yarn v3 IMO.

@brunomacf
Copy link
Author

@JounQin i've already run yarn dlx @yarnpkg/sdks vscode... as you can see here https://github.com/brunomacf/vscode-eslint-yarn-berry/blob/main/.vscode/settings.json#L6 it's already patching eslint. Very weird :(

@JounQin
Copy link

JounQin commented Jul 6, 2022

@brunomacf

It doesn't patch eslint-plugin-import.

@brunomacf
Copy link
Author

brunomacf commented Jul 6, 2022

@JounQin and as i mentioned above the weird thing is: if i run yarn eslint . --ext .ts in terminal then eslint-plugin-import seems to be working fine so it seems yarn is patching the env correctly for eslint to work.

Captura de Tela 2022-07-04 às 14 25 30

@brunomacf
Copy link
Author

@JounQin Indeed maybe yarn-sdks is not patching it correctly on vscode... I'm going to post this issue there!! 👍

@JounQin
Copy link

JounQin commented Jul 6, 2022

As I mentioned at #1494 (comment)

It's yarn's responsibility to fix this issue with vscode, not vscode-eslint nor eslint-plugin-import.

@lachlanhunt
Copy link

I think I'm also experiencing this same issue. I noticed in the VSCode ESLint Output channel, it shows this error:

[Info  - 12:58:00 pm] ESLint server is starting
[Info  - 12:58:00 pm] ESLint server running in node v16.13.2
[Info  - 12:58:00 pm] ESLint server is running.
[Info  - 12:58:01 pm] ESLint library loaded from: /Users/lhunt/Sites/sandbox/.yarn/sdks/eslint/lib/api.js
Uncaught exception received.
Error: Cannot find module '/Users/lhunt/Sites/sandbox/.yarn/__virtual__/eslint-import-resolver-typescript-virtual-948cc217d9/0/cache/eslint-import-resolver-typescript-npm-3.4.0-fc722c19cd-42fa7fbf7a.zip/node_modules/eslint-import-resolver-typescript/lib/worker.mjs'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:987:15)
    at Module._load (node:internal/modules/cjs/loader:832:27)
    at Function.c._load (node:electron/js2c/asar_bundle:5:13343)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at MessagePort.<anonymous> (node:internal/main/worker_thread:193:24)
    at MessagePort.[nodejs.internal.kHybridDispatch] (node:internal/event_target:562:20)
    at MessagePort.exports.emitMessage (node:internal/per_context/messageport:23:28)

I also ended up applying a similar fix to kriasoft/react-starter-kit#1984 to exclude the import/* plugins when running inside VSCode.

I also suspect (but can't confirm) this problem may be the root cause of import-js/eslint-import-resolver-typescript#163, which causes VSCode to get stuck while trying to save a file.

@JounQin
Copy link

JounQin commented Aug 6, 2022

@lachlanhunt See #1494 (comment)

@lachlanhunt
Copy link

lachlanhunt commented Aug 7, 2022

I investigated further and found that specific error I posted above comes from within synckit (a dependency of eslint-import-resolver-typescript), and is thrown from the worker thread it tries to create.

The import resolver is passing synckit the fully resolved path to worker.mjs, which is correctly resolved to be within the zip file by Yarn PnP. In my case, that's:

/Users/lhunt/Sites/sandbox/.yarn/__virtual__/eslint-import-resolver-typescript-virtual-948cc217d9/0/cache/eslint-import-resolver-typescript-npm-3.4.0-fc722c19cd-42fa7fbf7a.zip/node_modules/eslint-import-resolver-typescript/lib/worker.mjs

With Yarn PnP setup performed, if you give a path like this to require(...), it handles it correctly and loads it from the zip file. However, when constructing a new Worker(...), which synckit tries to do, that's not being handled correctly.

I found yarnpkg/berry#2476 which appears to be about this exact a similar issue. But it's had very little attention since it was filed in February 2021.

@JounQin
Copy link

JounQin commented Aug 7, 2022

@lachlanhunt See un-ts/synckit#98

@lachlanhunt
Copy link

In my testing, this issue has been resolved by updating to synckit 0.8.3. Thanks to un-ts/synckit#98 and un-ts/synckit#103

@JounQin
Copy link

JounQin commented Aug 10, 2022

@brunomacf Can you help to test whether it's working as expected after upgrading?

All you need to do is yarn add -D eslint-import-resolver-typescript

@dbaeumer
Copy link
Member

dbaeumer commented Dec 5, 2022

I will close the issue since according to the latest comments it got solved in the plugin.

@dbaeumer dbaeumer closed this as completed Dec 5, 2022
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

5 participants