Add an optional peer dependency to react-apollo

[zkochan]

@types/react should be an optional peer dependency of react-apollo.

Related issue pnpm/pnpm#1928

Checklist:

• If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests
• If this was a change that affects the external API used in GitHunt-React, update GitHunt-React and post a link to the PR in the discussion.

[apollo-cla]

@zkochan: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[hwillson]

Thanks for the PR @zkochan. Can you explain why @types/react needs to be a peer dep, when we already have it in place as a dev dep? pnpm/pnpm#1928 doesn't seem to explain the reason why.

[zkochan]

This package exports a React component which is extending the React component class. When this package is required by a typescript project, compilation fails because the typing of the react component is not found. Having it as a dev dependency does not fix the issue because dev dependencies are only installed during local development.

[hwillson]

Got it, I see that now - ApolloProvider is using the React.FC type from @types/react; when ApolloProvider's types are being picked up and @types/react isn't installed, it's causing grief. This hasn't been a widely reported issue, and we've had a lot of beta testers using TS. This makes sense, as most React + TS users will have @types/react already installed with their apps.

Hmmm ... I'll have to think about this a bit more. If we make @types/react a peer dep, then React Apollo users will have to know to manually install it to address issues like the one reported. Looking over the codebase, this issue can only happen with some of our exported entities, which means making @types/react a peer dep might be overkill. We might be able to adjust our code to break this dependency completely (without losing extra type details). I'll look into this further. Thanks!

[hwillson]

Thanks for this @zkochan!

[haleykoike]

@hwillson With this PR included in 3.0.1, this leads to an unfortunate side effect for non TS users. When upgrading to 3.0.1, I'm either forced to install and add @types/react into my own project's dependencies even though I don't use TS in my project, or I have to always have unmet peer dependency warnings cluttering npm install logs.

While neither is particularly serious, this isn't something that non TS users should need to deal with. You mentioned looking into adjusting code to break the dependency completely. Would this still be possible?

[zkochan]

It appears that this is an issue with the npm registry not returning the peerDependenciesMeta with the package metadata. See npm/cli#224 (comment)

When the registry will be updated, you will stop seeing the warnings.

Though with Yarn and pnpm there are no warnings already, I believe