Yarn v2 issue with @types

vhiairrassary commented 5 years ago

I am testing pnp-webpack-plugin with yarn v2 (2.0.0-rc.1, but same output with 2.0.0-rc.6).

It works well with JS: https://github.com/vhiairrassary/test-yarn-v2.

When I want to use typescript, I can't use @types packages. The code now is https://github.com/vhiairrassary/test-yarn-v2/compare/ts?expand=1.

Without adding @types/express, I get the following error, which is normal

ERROR in [...]/test-yarn-v2/packages/a/sources/index.ts
./sources/index.ts
[tsl] ERROR in [...]/test-yarn-v2/packages/a/sources/index.ts(1,26)
  TS7016: Could not find a declaration file for module 'express'. '[...]/test-yarn-v2/.yarn/cache/express-npm-4.17.1-e3afcd926c.zip/node_modules/express/index.js' implicitly has an 'any' type.

Adding to the package using yarn workspace @mytest/a add @types/express results in the same error as above, but here it should work out of the box.

Adding to the root package using yarn workspace @mytest/root add @types/express:

ERROR in [...]/test-yarn-v2/packages/a/sources/index.ts
./sources/index.ts
[tsl] ERROR in [...]/test-yarn-v2/packages/a/sources/index.ts(6,67)
  TS2339: Property 'send' does not exist on type 'Response'.

Which is really weird as send is part of Response.

Am I missing something?

arcanis commented 5 years ago

For the second one I remember seeing that before, but I haven't got time to investigate yet 🤔

For the third one, that looks weird as well but at least the types definition are found. My best guess would be that the version of @types/express-serve-static-core that got install doesn't list send (since @types/express depends on it through *, technically any version would match - including those without send).

vhiairrassary commented 5 years ago

Weird, with unzip -c .yarn/cache/@types-express-serve-static-core-npm-4.16.9-56b6aa787f.zip node_modules/@types/express-serve-static-core/index.d.ts I can see :

export interface Response extends http.ServerResponse, Express.Response {
    /**
     * Set status `code`.
     */
    status(code: number): Response;

    /**
     * Set the response HTTP status code to `statusCode` and send its string representation as the response body.
     * @link http://expressjs.com/4x/api.html#res.sendStatus
     *
     * Examples:
     *
     *    res.sendStatus(200); // equivalent to res.status(200).send('OK')
     *    res.sendStatus(403); // equivalent to res.status(403).send('Forbidden')
     *    res.sendStatus(404); // equivalent to res.status(404).send('Not Found')
     *    res.sendStatus(500); // equivalent to res.status(500).send('Internal Server Error')
     */
    sendStatus(code: number): Response;

    /**
     * Set Link header field with the given `links`.
     *
     * Examples:
     *
     *    res.links({
     *      next: 'http://api.example.com/users?page=2',
     *      last: 'http://api.example.com/users?page=5'
     *    });
     */
    links(links: any): Response;

    /**
     * Send a response.
     *
     * Examples:
     *
     *     res.send(new Buffer('wahoo'));
     *     res.send({ some: 'json' });
     *     res.send('<p>some html</p>');
     *     res.status(404).send('Sorry, cant find that');
     */
    send: Send;

4.16.9 is actually the latest version listed on npm: https://www.npmjs.com/package/@types/express-serve-static-core

vhiairrassary commented 5 years ago

For the second one, with PNP_DEBUG_LEVEL=5 we can see @types/express is required by @mytest/root@workspace:. which is not the correct package:

{
  fn: 'resolveToUnqualified',
  args: [
    '@types/express',
    '[...]/test-yarn-v2/',
    { considerBuiltins: false }
  ],
  error: Error: A package is trying to access another package without the second one being listed as a dependency of the first one

  Required package: @types/express (via "@types/express")
  Required by: @mytest/root@workspace:. (via [...]/test-yarn-v2/)

      at Object.makeError ([...]/test-yarn-v2/.pnp.js:6841:26)
      at resolveToUnqualified ([...]/test-yarn-v2/.pnp.js:9248:43)
      at [...]/test-yarn-v2/.pnp.js:9323:32
      at Object.resolveToUnqualified ([...]/test-yarn-v2/.pnp.js:8937:50)
      at resolveModuleName ([...]/test-yarn-v2/.yarn/virtual/ts-pnp-virtual-11ad59ec2c/0/cache/ts-pnp-npm-1.1.4-68c6fd0fd3.zip/node_modules/ts-pnp/index.js:22:25)
      at [...]/test-yarn-v2/.yarn/virtual/ts-loader-virtual-081796e733/0/cache/ts-loader-npm-6.1.2-234ca82b1e.zip/node_modules/ts-loader/dist/servicesHost.js:412:44
      at resolveModule ([...]/test-yarn-v2/.yarn/virtual/ts-loader-virtual-081796e733/0/cache/ts-loader-npm-6.1.2-234ca82b1e.zip/node_modules/ts-loader/dist/servicesHost.js:391:26)
      at [...]/test-yarn-v2/.yarn/virtual/ts-loader-virtual-081796e733/0/cache/ts-loader-npm-6.1.2-234ca82b1e.zip/node_modules/ts-loader/dist/servicesHost.js:98:63
      at Array.map (<anonymous>)
      at Object.resolveModuleNames ([...]/test-yarn-v2/.yarn/virtual/ts-loader-virtual-081796e733/0/cache/ts-loader-npm-6.1.2-234ca82b1e.zip/node_modules/ts-loader/dist/servicesHost.js:98:45)
      at Object.compilerHost.resolveModuleNames ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:125749:52)
      at resolveModuleNamesWorker ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:91510:127)
      at resolveModuleNamesReusingOldState ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:91756:24)
      at processImportedModules ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:93095:35)
      at findSourceFile ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:92896:17)
      at [...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:92742:85
      at getSourceFileFromReferenceWorker ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:92709:34)
      at processSourceFile ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:92742:13)
      at processRootFile ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:92572:13)
      at [...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:91588:60
      at Object.forEach ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:288:30)
      at Object.createProgram ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:91588:16)
      at synchronizeHostData ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:125769:26)
      at Object.getProgram ([...]/test-yarn-v2/.yarn/cache/typescript-npm-3.6.3-77c6d05fb2.zip/node_modules/typescript/lib/typescript.js:125861:13)
      at successfulTypeScriptInstance ([...]/test-yarn-v2/.yarn/virtual/ts-loader-virtual-081796e733/0/cache/ts-loader-npm-6.1.2-234ca82b1e.zip/node_modules/ts-loader/dist/instances.js:179:80)
      at Object.getTypeScriptInstance ([...]/test-yarn-v2/.yarn/virtual/ts-loader-virtual-081796e733/0/cache/ts-loader-npm-6.1.2-234ca82b1e.zip/node_modules/ts-loader/dist/instances.js:34:12)
      at Object.loader ([...]/test-yarn-v2/.yarn/virtual/ts-loader-virtual-081796e733/0/cache/ts-loader-npm-6.1.2-234ca82b1e.zip/node_modules/ts-loader/dist/index.js:17:41)
      at LOADER_EXECUTION ([...]/test-yarn-v2/.yarn/cache/loader-runner-npm-2.4.0-1dda35a78d.zip/node_modules/loader-runner/lib/LoaderRunner.js:119:14)
      at runSyncOrAsync ([...]/test-yarn-v2/.yarn/cache/loader-runner-npm-2.4.0-1dda35a78d.zip/node_modules/loader-runner/lib/LoaderRunner.js:120:4)
      at iterateNormalLoaders ([...]/test-yarn-v2/.yarn/cache/loader-runner-npm-2.4.0-1dda35a78d.zip/node_modules/loader-runner/lib/LoaderRunner.js:232:2) {
    code: 'MODULE_NOT_FOUND',
    pnpCode: 'UNDECLARED_DEPENDENCY',
    data: {
      request: '@types/express',
      issuer: '[...]/test-yarn-v2/',
      issuerLocator: [Object],
      dependencyName: '@types/express',
      candidates: [Array]
    }
  },
  result: null
}

vhiairrassary commented 5 years ago

I suspect the answer might be in ts-pnp: https://github.com/arcanis/ts-pnp/blob/master/index.js. But I don't know how to monkey patch (as packages are stored as zip files) or debug (inspect-brk breaks the debugger with "NODE_OPTIONS='--require [...]/test-yarn-v2/.pnp.js --inspect --inspect-brk' webpack-cli --progress").

Do you know what is the recommended way to debug in such situations?

arcanis commented 5 years ago

Two options:

You can edit directly within the zip archives with Emacs or Vim (too bad VSCode doesn't have this neat feature :().
You can "unplug" the package you want to edit with yarn unplug <name>. It will be stored inside .yarn/unplugged/<package-name> and you can edit it at will.

I think you're right btw - looking at this line, it's clear that the resolution is made from the top-level of the project (which is obtained from pnp.topLevel). I'm not 100% sure I remember whether I did it on purpose or if it's a bug 🤔

vhiairrassary commented 5 years ago

Two options:

You can edit directly within the zip archives with Emacs or Vim (too bad VSCode doesn't have this neat feature :().

You can "unplug" the package you want to edit with yarn unplug <name>. It will be stored inside .yarn/unplugged/<package-name> and you can edit it at will.

unplug: Awesome 🎉

I think you're right btw - looking at this line, it's clear that the resolution is made from the top-level of the project (which is obtained from pnp.topLevel). I'm not 100% sure I remember whether I did it on purpose or if it's a bug 🤔

I confirm. At https://github.com/arcanis/ts-pnp/blob/master/index.js#L22, Replacing

pnp.resolveToUnqualified(typesPackagePath, `${topLevelLocation}/`, {considerBuiltins: false});

by

pnp.resolveToUnqualified(typesPackagePath, issuer, {considerBuiltins: false});

fix my issue and the example compiles as expected! But I am not confident in creating a PR for the issue 😅

arcanis commented 5 years ago

I vaguely remember that it was because of an oddity with the way the @types dependencies depend on one another ... it's a bit fuzzy (I should have wrote a test about it!), but iirc they don't really work if there's multiple times the same @types package in the same project with different versions. By only resolving them from the root, we never have this problem.

arseneyr commented 4 years ago

I'm running into this same issue by trying to run a stock create-react-app in a workspace with yarn 2.0.0-rc.19. Sample repo is here: https://github.com/arseneyr/cra-pnp-workspaces

I get errors about @types/react not found. Unplugging ts-pnp and using the following workaround suggested by @vhiairrassary gets rid of these:

I confirm. At https://github.com/arcanis/ts-pnp/blob/master/index.js#L22, Replacing
pnp.resolveToUnqualified(typesPackagePath, `${topLevelLocation}/`, {considerBuiltins: false});
by
pnp.resolveToUnqualified(typesPackagePath, issuer, {considerBuiltins: false});

Now I get errors about @types/node missing. Running with PNP_DEBUG_LEVEL=5 shows the following problem:

{
  fn: 'resolveToUnqualified',
  args: [
    '@types/node',
    '[...]/cra-pnp-workspaces/.yarn/virtual/react-scripts-virtual-dc9c9137a7/0/cache/react-scripts-npm-3.3.0-08a5d5e86b-1.zip/node_modules/react-scripts/lib/react-app.d.ts',
    { considerBuiltins: false }
  ],
  error: Error: A package is trying to access another package without the second one being listed as a dependency of the first one

  Required package: @types/node (via "@types/node")
  Required by: react-scripts@virtual:b8882b4b8798764a038d960ea6930735d737ef43bc80fa7092e9d35d44be4e2eb8b4b456600862280c38ea553cdf19dbbe483c521fde73d6cefe05d9f4340e03#npm:3.3.0 (via /[...]/cra-pnp-workspaces/.yarn/virtual/react-scripts-virtual-dc9c9137a7/0/cache/react-scripts-npm-3.3.0-08a5d5e86b-1.zip/node_modules/react-scripts/lib/react-app.d.ts)

      at internalTools_makeError ([...]\cra-pnp-workspaces\.pnp.js:24263:24)
      at resolveToUnqualified ([...]\cra-pnp-workspaces\.pnp.js:25243:19)
      at [...]\cra-pnp-workspaces\.pnp.js:25407:26
      at Object.resolveToUnqualified ([...]\cra-pnp-workspaces\.pnp.js:24872:38)
      at resolveModuleName ([...]\cra-pnp-workspaces\.yarn\unplugged\ts-pnp-virtual-aabe5b8aaf\node_modules\ts-pnp\index.js:22:25)
      at exports.resolveTypeReferenceDirective ([...]\cra-pnp-workspaces\.yarn\virtual\react-scripts-virtual-dc9c9137a7\0\cache\react-scripts-npm-3.3.0-08a5d5e86b-1.zip\node_modules\react-scripts\config\pnpTs.js:36:10)
      at [...]\cra-pnp-workspaces\.yarn\cache\fork-ts-checker-webpack-plugin-npm-3.1.0-78e057ec7a-1.zip\node_modules\fork-ts-checker-webpack-plugin\lib\CompilerHost.js:52:28
      at Array.map (<anonymous>)
      at CompilerHost.resolveTypeReferenceDirectives ([...]\cra-pnp-workspaces\.yarn\cache\fork-ts-checker-webpack-plugin-npm-3.1.0-78e057ec7a-1.zip\node_modules\fork-ts-checker-webpack-plugin\lib\CompilerHost.js:51:43)
      at Object.resolveTypeReferenceDirectives ([...]\cra-pnp-workspaces\.yarn\cache\typescript-npm-3.7.3-74e8e31e02-1.zip\node_modules\typescript\lib\typescript.js:100318:60) {
    code: 'MODULE_NOT_FOUND',
    pnpCode: 'UNDECLARED_DEPENDENCY',
    data: {
      request: '@types/node',
      issuer: '/[...]/cra-pnp-workspaces/.yarn/virtual/react-scripts-virtual-dc9c9137a7/0/cache/react-scripts-npm-3.3.0-08a5d5e86b-1.zip/node_modules/react-scripts/lib/react-app.d.ts',
      issuerLocator: [Object],
      dependencyName: '@types/node',
      candidates: [Array]
    }
  },
  result: null
}

This looks like a problem with react-scripts not declaring @types/node as a peerDependency. Manually editing the yarn.lock file to reflect this dependency fixes the issue and the CRA works as expected. The strange thing is that a non-workspaced CRA does not have this issue. The issuer for @types/node in that case is just the root package:

{
  fn: 'resolveToUnqualified',
  args: [
    '@types/node',
    '[...]\\cra-pnp/',
    { considerBuiltins: false }
  ],
  error: null,
  result: '[...]\\cra-pnp\\.yarn\\cache\\@types-node-npm-12.12.20-4545b71073-1.zip\\node_modules\\@types\\node'
}

Could you please shed some light on these questions?

Should ts-pnp get fixed to resolve from the issuer instead of the root? Or do we have to manually install the @types/* packages in the project root?
(Unrelated to this issue but maybe you have insight) Should react-scripts be declaring @types/node as a peerDependency and why does it just work in the non-workspaced case?

walkerburgin commented 4 years ago

I hit this as well while experimenting a bit with yarn v2 to get a feel for how difficult migration will ultimately be. Unplugging and patching ts-pnp as described here got me unblocked (thank you!).

Or do we have to manually install the @types/* packages in the project root?

If this turns out to be the case it would be a pretty hard blocker for us being able to adopt v2. We have a pretty massive TypeScript monorepo (hundreds of packages) and rely heavily on each of them declaring their @types dev dependencies independently so that we can reason about it.

nmocruz commented 4 years ago

This is a big blocker to me.

alubbe commented 4 years ago

The @types issue is solved by changing the above lines, but (with or without this change) TS cannot find the normal type files for packages that publish their own typescript files - see: https://github.com/yarnpkg/berry/issues/1483

I'm not 100% sure where to post about this issue - does anyone know or have an idea of how to tackle it?

alubbe commented 4 years ago

Just to clarify, my issue can be reproduced by cloning https://github.com/alubbe/yarn2_workspace_typescript and running yarn install && cd packages/rofl && yarn start. And all it takes to break it is to go from a simple repo to a monorepo with these simple changes: https://github.com/alubbe/yarn2_workspace_typescript/commit/7839493116b4df41a7dfd328b9819116d20d1cff

alubbe commented 4 years ago

I hate to bump my own ticket, but this issue is causing major headaches for us - is there anything we can do to help investigate this further? Is this even the right place to report the issue? Why does TS find all the JS files just fine, but struggle to find a dependency's own type declaration file? Any pointers would be extremely helpful!

alubbe commented 4 years ago

Fixed by https://github.com/yarnpkg/berry/issues/1483#issuecomment-663939833

arcanis / pnp-webpack-plugin

Yarn v2 issue with @types #14