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

react and graphql-tag dependency adjustments #5451

Merged
merged 10 commits into from
Oct 15, 2019

Conversation

hwillson
Copy link
Member

@hwillson hwillson commented Oct 14, 2019

Includes the following:

  • Makes react an optional dependency (since @apollo/client's React integration can be removed using tree-shaking / dead code elimination)
  • Updates React related dependencies
  • Makes graphql-tag a full dependency, and re-exports gql making it part of the default @apollo/client bundle (to pave the way for future gql optimizations)
  • Updates the docs to import gql from @apollo/client

We're going to re-export `gql`, making it part of the
`@apollo/client` bundle, to pave the way for future optimizations
(that will require `gql` being part of core).

Note: Due to the way `graphql-tag` is bundled, we have to follow
Rollup's recommendations listed at
https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module
to be able to include it in the `@apollo/client` bundle.
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but we need to include graphql-tag in the globals map in config/rollup.config.js to avoid re-bundling graphql-tag into the @apollo/client bundle(s), which is what caused the bundle size to spike so dramatically. I'll push a commit to fix that in a moment.

package.json Outdated Show resolved Hide resolved
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to re-run those tests once Circle CI resolves the problems with docker containers, but I think this is ready to merge, otherwise.

@benjamn benjamn merged commit ac1c149 into release-3.0 Oct 15, 2019
@benjamn benjamn deleted the dependency-adjustments branch October 15, 2019 17:24
benjamn added a commit that referenced this pull request Oct 15, 2019
package.json Show resolved Hide resolved
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants