[Bug?]: pnp is unusable in TypeScript monorepos

adrian-gierakowski commented 9 months ago

Self-service

[ ] I'd be willing to implement a fix

Describe the bug

When using pnp linker in a TypeScript monorepo, auto-import fails for symbols from a workspace: dep which in turn has peerDependencies (and hence is virtualized).

Note that auto-complete works (so LSP seems to be able to compile all package just fine), but when you accept a completion the symbol is not not imported and LSP logs show an error like this:

<semantic> TypeScript Server Error (5.3.3-sdk)
Debug Failure.
Error: Debug Failure.
    at getAutoImportSymbolFromCompletionEntryData (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:160638:150)
    at getSymbolCompletionFromEntryId (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:158968:24)
    at Object.getCompletionEntryDetails (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:159020:28)
    at Object.getCompletionEntryDetails2 [as getCompletionEntryDetails] (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:145308:35)
    at /home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:184984:43
    at mapDefined (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:2668:22)
    at IpcIOSession.getCompletionEntryDetails (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:184982:20)
    at completionEntryDetails (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:183311:43)
    at /home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:185690:69
    at IpcIOSession.executeWithRequestId (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:185682:14)
    at IpcIOSession.executeCommand (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:185690:29)
    at IpcIOSession.onMessage (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:185732:51)
    at IpcIOSession.onMessage (/home/me/code/my-project/.yarn/sdks/typescript/lib/tsserver.js:201:32)
    at process.<anonymous> (/home/me/.yarn/berry/cache/typescript-patch-71fdfda6a2-10c0.zip/node_modules/typescript/lib/tsserver.js:187322:14)
    at process.emit (node:events:513:28)
    at process.emit (/home/me/code/my-project/.pnp.cjs:70764:25)
    at emit (node:internal/child_process:937:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:83:21)

To reproduce

Full repro with instructions can be found in this repo.

Environment

Specific version are listed in in the repo mentioned above, although this has been reproduced with other versions of node, tsc and with different editors (vscode, vscodium and submile-text 4 with https://lsp.sublimetext.io/language_servers/ extension).

Additional context

adrian-gierakowski commented 8 months ago

Any ideas what could be causing this?

adrian-gierakowski commented 8 months ago

Would be great if maintainers could comment if there was any chance for this issue (and related https://github.com/yarnpkg/berry/issues/3829, which has been open since over 2 years) to receive any attention and eventually be fixed.

As it stands now, yarn berry seems unsuitable for use in TypeScript monorepos.

adrian-gierakowski commented 3 months ago

still an issue with typescript 5.4.5 and yarn 4.1.1

this has also been reported on stackoverflow: https://stackoverflow.com/questions/78034031/imports-in-yarn-monorepo-are-suggested-correctly-but-not-entered-when-selected

no wonder so many devs are choosing pnpm these days

SheepReaper commented 2 months ago

adrian-gierakowski commented 2 months ago

@SheepReaper thanks, but I dot think so. The problem described here applies specifically to auto imports from local workspace package with peer dependencies (but LSP works in general in the repro project)

In your case, you simple need to make sure that you are using version of typescript which is supported by the version of yarn you are using (which your repro doesn’t specify btw). Yarn berry ships with patches for typescript which are needed for it to work with pnp, and usually latest versions of typescript don’t work for a while after they are released since yarn lags a bit with releases of those patches.

SheepReaper commented 2 months ago

@adrian-gierakowski I am unable to reproduce your error in your repo because the error I've reported in mine prevents getting that far.

The yarn version is specified, it's in the packageManager field in package.json. yarn 4 doesn't use yarnPath anymore, it uses corepack. Also, you can see the patch being downloaded and applied if you remove/re-add or do an --exact version install of typescript. If there's an error in the patch application, it shows up there. All that is to say it's the latest yarn and latest typescript, and a patch does exist and has been applied. Finally, running yarn tsc --build proves everything is working correctly but leaves me scratching my head as to why my issue manifests only in the ui.

What I see in common between our issues is that yarn is using the pnp linker. The moment I switch to node-modules, your issue goes away too: second import works too

yarnpkg / berry