Berry: [Bug] yarn pnpify tsc can not resolve some graphql types

[mohsen1]

Describe the bug

See my PR trying to use P'n'P on this simple project that uses TypeScript and Webpack:

https://github.com/mohsen1/apollo-react-async-ssr/pull/3

I get errors like the following.

note

What's significant about Apollo packages is that they ship their own types.

.yarn/cache/apollo-server-plugin-base-npm-0.5.6-cc277148df89efcb60033ea282a14f7c47f09911109e741cf6c7c6ccce45bc57.zip/node_modules/apollo-server-plugin-base/dist/index.d.ts:1:95 - error TS2307: Cannot find module 'apollo-server-core/dist/requestPipelineAPI'.

1 import { GraphQLServiceContext, GraphQLRequestContext, GraphQLRequest, GraphQLResponse } from 'apollo-server-core/dist/requestPipelineAPI';
                                                                                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.yarn/cache/graphql-extensions-npm-0.7.4-c0f20210a7443737d662bd6d508e284683a78027a2816cb42cade84f293591da.zip/node_modules/graphql-extensions/dist/index.d.ts:2:25 - error TS2307: Cannot find module 'apollo-server-env'.

2 import { Request } from 'apollo-server-env';
                          ~~~~~~~~~~~~~~~~~~~

Environment:

• OS: Linux
• Node version 10
• Yarn version 2

Additional context

tsc does pass with the code in master.

[arcanis]

I believe this is a apollo-server-plugin-base problem - they only depend on apollo-server-core as a dev dependency, so the imported types cannot be found. Cf here:

https://cdn.jsdelivr.net/npm/[email protected]/package.json

[mohsen1]

How this was working before? 🤔

[arcanis]

My guess is that some other package depended on apollo-server-core and, by the magic of the hoisting, apollo-server-plugin-base was stealing it. PnP enforces proper package boundaries (a package can only access its dependencies), so situations like this can't happen.

[JacksonKearl]

apollo-server-plugin-base only depends on apollo-server-core for typings (makes sense, as the entire apollo-server-plugin-base is just typings), and as such does not require apollo-server-core outside of a dev environment. I am not very familiar with standard procedure when it comes to type-only packages, what would be appropriate here?

[mohsen1]

It is advised that packages never depend to their @types/* packages as a direct or a peer dependency because of various issues that sort of dependency causes.

Since Apollo packages don't require a type dependency it's a bit different. I am not sure what is the right thing to do here.

Installing apollo-server-core as a direct dependency in my _TypeScript compiled_ package will solve my issue. But this is a bad DX. It took a while to find this out.

Maybe TypeScript should have a concept of "type dependencies" and Yarn can install those if TypeScript is involved...

/cc @DanielRosenwasser

[arcanis]

IMO, generally speaking, the @types packages should list peer dependencies, not regular dependencies. This has been my stance for quite some time:

• https://github.com/yarnpkg/berry/issues/3#issuecomment-503473949
• https://github.com/yarnpkg/yarn/issues/3951
• https://github.com/yarnpkg/yarn/issues/6695

In the particular case of Apollo, it seems like things should work better with using a regular dependency (because unless I'm mistaken it doesn't export global symbols; if it did then a peer dependency would be safer).

---

Maybe TypeScript should have a concept of "type dependencies" and Yarn can install those if TypeScript is involved...

We have an experimental TypeScript plugin that would be the perfect ground for such things (right now it simply auto-adds the corresponding @types packages when you add something else). Of course it wouldn't work with npm 🤷‍♀️

[arcanis]

I'll close this issue as it seems caused by an improper dependency listing in the Apollo package.

[CallMeLaNN]

@arcanis Does this related to https://github.com/apollographql/apollo-server/issues/3222 ?

My theory is that pnpify can't resolve @types/* of child packages that being use by package maintainer. I just created this repo to reproduce the error.

apollo-server-express > @types/express > @types/express-serve-static-core

I do have the first two because I'm using it. Since Yarn flattened all dependencies on the tree, I also have .yarn/cache/@types-express-serve-static-core...zip. but I have to add @types/express-serve-static-core into my devDep too in order to update .pnp.js and make it work.

Update: apollo-server-express avoid that by explicitly use type exported by @types/express