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

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

Open
adrian-gierakowski opened this issue Nov 2, 2022 · 13 comments

Comments

@adrian-gierakowski
Copy link

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

artola commented Jan 13, 2023

@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?
typescript-eslint/typescript-eslint#5892 (comment)

@adrian-gierakowski
Copy link
Author

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

@Quramy
Copy link
Owner

Quramy commented Apr 6, 2023

Alternatively would you accept a PR?

👍

@artola
Copy link
Contributor

artola commented Apr 6, 2023

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

Quramy commented Apr 6, 2023

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

artola commented Apr 6, 2023

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

@JoshuaKGoldberg
Copy link

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

artola commented Apr 7, 2023

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

  •   parserOptions: {
        project: "./tsconfig.json",
        tsconfigRootDir: __dirname,
      },
    
  •   parserOptions: {
        project: true,
      },
    

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

Thanks @Quramy for your support.

CC @awydler

@adrian-gierakowski
Copy link
Author

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

artola commented Apr 8, 2023

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

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

JoshuaKGoldberg commented Apr 8, 2023

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 (typescript-eslint/typescript-eslint#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
Copy link
Contributor

artola commented Apr 8, 2023

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.

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

4 participants