Quramy / typescript-eslint-language-service

TypeScript language service plugin for ESLint
MIT License
252 stars 10 forks source link

error when using pnp in strict mode due to undeclared dependencies #497

Open adrian-gierakowski opened 1 year ago

adrian-gierakowski commented 1 year ago

for example:

Failed to load module 'typescript-eslint-language-service' from /home/adrian/code/test-project/node_modules: Error: typescript-eslint-language-service tried to access @typescript-eslint/scope-manager, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

This affects users of yarn 3 (berry) and https://pnpm.io when using pnp.

Here's a patch which I had to put in my .yarnrc.yml to make silence the errors, which shows the deps accessed by this module in an unsound manner:

packageExtensions:
  typescript-eslint-language-service@*:
    dependencies:
      "@eslint/eslintrc": ^1.3.3
      "@typescript-eslint/scope-manager": "*"
      "@typescript-eslint/types": "*"
      "@typescript-eslint/typescript-estree": "5.41.0"
      "@typescript-eslint/visitor-keys": "*"
      globby: ^11.1.0
      is-glob: ^4.0.3

Note that I had to pin @typescript-eslint/typescript-estree to 5.41.0 since 5.42.0 introduced a breaking change which breaks typescript-eslint-language-service

artola commented 1 year ago

@Quramy This plugin is AWESOME! do you plan to update it to support the mentioned above breaking change in dependency 5.42.0? or wait to v6 because of this comment? https://github.com/typescript-eslint/typescript-eslint/pull/5892#discussion_r1012924718

adrian-gierakowski commented 1 year ago

Any plan to have this fixed? Alternatively would you accept a PR?

Quramy commented 1 year ago

Alternatively would you accept a PR?

👍

artola commented 1 year ago

@Quramy While the e2e tests are all passing, I am not able to see the reported errors in VSCode, while the linter is able to find them.

These are the last valid dependencies known where errors are reported:

    "@typescript-eslint/parser": "5.41.0",
    "eslint": "^8.37.0",
    "typescript": "^4.9.5",
    "typescript-eslint-language-service": "5.0.0"

Changing typescript to v5 or typescript-eslint-language-service to other greater than 5.0.0 does not show the errors.

For example, with these newer dependencies:

    "typescript": "^5.0.3",
    "typescript-eslint-language-service": "^5.0.5"

Here a small repo: https://github.com/artola/typescript-eslint-language-service-issue.git

While running the script yarn lint reports:

[some-folder] % yarn lint     

/some-folder/src/main.ts
  1:1   error  Unexpected 'debugger' statement      no-debugger
  2:12  error  There should be no space after '{'   object-curly-spacing
  2:22  error  There should be no space before '}'  object-curly-spacing
  2:26  error  Missing semicolon                    semi

✖ 4 problems (4 errors, 0 warnings)
  3 errors and 0 warnings potentially fixable with the `--fix` option.
Quramy commented 1 year ago

@artola

Here a small repo: https://github.com/artola/typescript-eslint-language-service-issue.git

I tried to reproduce using your repos.

I confirmed reproducing that there're no errors reported in VSC after installing TypeScript v5 and typescript-eslint-language-service v5.0.5 .

However, ESLint errors are still reported to my editor(Vim). My Vim uses https://github.com/dense-analysis/ale and https://github.com/Quramy/tsuquyomi , they interact with tsserver.

image

I don't know why only VSC can't report ESLint errors 🤔

artola commented 1 year ago

@JoshuaKGoldberg could you please give us some hint about what could be the breaking change?

JoshuaKGoldberg commented 1 year ago

I don't use typescript-eslint-language-service so no, I don't know what's going on. 😄

If you think it's an issue with @typescript-eslint/parser then I'd encourage you to file an issue on typescript-eslint over at https://github.com/typescript-eslint/typescript-eslint/issues/new/choose.

artola commented 1 year ago

@Quramy If I update the dependencies to the latest:

  "devDependencies": {
    "@typescript-eslint/parser": "^5.57.1",
    "eslint": "^8.37.0",
    "typescript": "^5.0.3",
    "typescript-eslint-language-service": "^5.0.5"
  },

I can see in the VSCode's tsserver.log (command: Typescript: Open TS Server log) the following log for the main.ts file:

Info 685  [19:58:43.239] [typescript-eslint-language-service] Cannot read file '/tsconfig.json'.
Info 686  [19:58:43.239] [typescript-eslint-language-service] Error: Cannot read file '/tsconfig.json'.
    at Object.diagnosticReporter [as onUnRecoverableConfigFileDiagnostic] (/[redacted]/typescript-eslint-language-service-issue/node_modules/@typescript-eslint/typescript-estree/dist/create-program/getWatchProgramsForProjects.js:99:11)
    at getParsedCommandLineOfConfigFile (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/typescript.js:35635:12)
    at parseConfigFile2 (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/typescript.js:122928:34)
    at Object.createWatchProgram (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/typescript.js:122534:7)
    at createWatchProgram (/[redacted]/typescript-eslint-language-service-issue/node_modules/@typescript-eslint/typescript-estree/dist/create-program/getWatchProgramsForProjects.js:286:22)
    at getWatchProgramsForProjects (/[redacted]/typescript-eslint-language-service-issue/node_modules/@typescript-eslint/typescript-estree/dist/create-program/getWatchProgramsForProjects.js:181:30)
    at createProjectProgram (/[redacted]/typescript-eslint-language-service-issue/node_modules/@typescript-eslint/typescript-estree/dist/create-program/createProjectProgram.js:54:95)
    at getProgramAndAST (/[redacted]/typescript-eslint-language-service-issue/node_modules/@typescript-eslint/typescript-estree/dist/parser.js:36:89)
    at parseAndGenerateServices (/[redacted]/typescript-eslint-language-service-issue/node_modules/@typescript-eslint/typescript-estree/dist/parser.js:140:11)
    at parseForESLint (/[redacted]/typescript-eslint-language-service-issue/node_modules/@typescript-eslint/parser/dist/parser.js:90:80)
    at ESLintAdapter.convertToESLintSourceCode (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript-eslint-language-service/lib/eslint-adapter.js:90:90)
    at ESLintAdapter.getESLintResult (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript-eslint-language-service/lib/eslint-adapter.js:117:33)
    at ESLintAdapter.getSemanticDiagnostics (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript-eslint-language-service/lib/eslint-adapter.js:138:39)
    at TemplateLanguageServiceProxy.adaptDiagnosticsCall (/Users/martin/.vscode/extensions/styled-components.vscode-styled-components-1.7.8/node_modules/typescript-template-language-service-decorator/lib/template-language-service-decorator.js:288:33)
    at Proxy.<anonymous> (/Users/martin/.vscode/extensions/styled-components.vscode-styled-components-1.7.8/node_modules/typescript-template-language-service-decorator/lib/template-language-service-decorator.js:54:25)
    at IpcIOSession.semanticCheck (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:178516:125)
    at /[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:178577:14
    at MultistepOperation.executeAction (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:177242:9)
    at /[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:177222:69
    at IpcIOSession.executeWithRequestId (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:180226:14)
    at Object.executeWithRequestId (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:178262:57)
    at Immediate._onImmediate (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:177222:26)
    at process.processImmediate (node:internal/timers:466:21)

Then it is clear that the problem is about finding /tsconfig.json. A bit of debugging and research leads me to: https://typescript-eslint.io/linting/typed-linting/

And "voilà" the solution is to tweak the .eslintrc.js in any of the following ways:

Now errors are properly display on VSCode. I hope this help other users.

Thanks @Quramy for your support.

CC @awydler

adrian-gierakowski commented 1 year ago

@artola would you be able to explain how what you wrote relates to the issue I have reported? The issue is about dependencies imported by this module not being listed in it’s package.json

artola commented 1 year ago

@adrian-gierakowski Since version 5.0.1 it is not used anymore @typescript-eslint/typescript-estree in this plugin, it is pulled as a transitive dependency by @typescript-eslint/parser but not accessed elsewhere. Then, the issue does not persist when moving forward.

[typescript-eslint-language-service-issue] % yarn why @typescript-eslint/typescript-estree
└─ @typescript-eslint/parser@npm:5.57.1 [c5205]
   └─ @typescript-eslint/typescript-estree@npm:5.57.1 [0a26c] (via npm:5.57.1 [0a26c])

There is another correlation between the dependencies and my findings. The problem is that as you mention, the semver as applied in version 5.0.0 does not guarantee matching dependencies, and such dependencies leads to some incompatibility at 5.42.0. IMO what it is wrong in this issue is that @typescript-eslint/typescript-estree introduces a breaking change in 5.42.0 without changing the version to 6.x.

Now, if we move all the dependencies to the latests versions, everything works. At that point, pinning dependencies in the plugin is not needed, could be improved with a simple comment/warning in the README.md.

adrian-gierakowski commented 1 year ago

@artola thats for taking the time to explain! I’ll try it out on my project with yarn berry once I’m back to work next week and report back if it works for me.

JoshuaKGoldberg commented 1 year ago

IMO what it is wrong in this issue is that @typescript-eslint/typescript-estree introduces a breaking change in 5.42.0 without changing the version to 6.x.

breaking change

Indigo Montoya meme: "you keep using that word... I do not think it means what you think it means"

https://hynek.me/articles/semver-will-not-save-you

https://www.hyrumslaw.com

The change in typescript-eslint (https://github.com/typescript-eslint/typescript-eslint/pull/5834) was an internal refactor that didn't change the public API. There's no reason why it would have been labeled as a breaking change. It's inaccurate to call it a breaking change. Please don't throw that shade at us. 😉

If typescript-eslint has a bug, then that's another story. Please file an issue on us.

But: what is the actual behavior change in @typescript-eslint/typescript-estree that you believe impacts this package? The OP links to an internal function -ensureAbsolutePath- not exported to consumers.


I tried cloning the repro repo, bumping @typescript-eslint/parser to 5.42, and restarting ESLint & TS servers but don't see any issues. What are the explicit steps to reproduce this issue?

artola commented 1 year ago

I tried cloning the repro repo, bumping @typescript-eslint/parser to 5.42, and restarting ESLint & TS servers but don't see any issues. What are the explicit steps to reproduce this issue?

@JoshuaKGoldberg Just set these dependencies and errors are not reported anymore (just change "@typescript-eslint/parser": "5.42.0",):

 "devDependencies": {
    "@typescript-eslint/parser": "5.42.0",
    "eslint": "^8.37.0",
    "typescript": "^4.9.5",
    "typescript-eslint-language-service": "5.0.0"
  }, 

Here the tsserver.log after reloading VSCode:

Info 99   [17:01:32.611] event:
    {"seq":0,"type":"event","event":"syntaxDiag","body":{"file":"/[redacted]/typescript-eslint-language-service-issue/src/main.ts","diagnostics":[]}}
Info 100  [17:01:32.611] [typescript-eslint-language-service] The "path" argument must be of type string. Received an instance of Object
Info 101  [17:01:32.611] [typescript-eslint-language-service] TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received an instance of Object
    at new NodeError (node:internal/errors:371:5)
    at validateString (node:internal/validators:120:11)
    at Object.join (node:path:1172:7)
    at ensureAbsolutePath (/[redacted]/typescript-eslint-language-service-issue/node_modules/@typescript-eslint/typescript-estree/dist/create-program/shared.js:71:26)
    at applyParserOptionsToExtra (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript-eslint-language-service/lib/ast-converter.js:156:54)
    at AstConverter.parseAndGenerateServices (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript-eslint-language-service/lib/ast-converter.js:215:21)
    at AstConverter.parseForESLint (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript-eslint-language-service/lib/ast-converter.js:280:44)
    at AstConverter.convertToESLintSourceCode (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript-eslint-language-service/lib/ast-converter.js:296:67)
    at ESLintAdapter.getESLintResult (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript-eslint-language-service/lib/eslint-adapter.js:95:43)
    at ESLintAdapter.getSemanticDiagnostics (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript-eslint-language-service/lib/eslint-adapter.js:116:39)
    at Session.semanticCheck (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:180735:52)
    at /[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:180801:31
    at MultistepOperation.executeAction (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:179660:25)
    at /[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:179639:100
    at Session.executeWithRequestId (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:182324:28)
    at Object.executeWithRequestId (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:180499:87)
    at Immediate._onImmediate (/[redacted]/typescript-eslint-language-service-issue/node_modules/typescript/lib/tsserver.js:179639:41)
    at process.processImmediate (node:internal/timers:466:21)

About "breaking change" or not, it’s not you it’s me. 😉 Something breaks in this ecosystem at that point. Anyway, moving all the dependencies forward and changing the .eslintrc.js as mentioned above everything works.

Then, the problem does not persist with the mentioned solution, and I don't know why this was affecting VSCode but not other solution as @Quramy mentioned above.