-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Conversation
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.
There was a problem hiding this 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.
There was a problem hiding this 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.
Includes the following:
react
an optional dependency (since@apollo/client
's React integration can be removed using tree-shaking / dead code elimination)graphql-tag
a full dependency, and re-exportsgql
making it part of the default@apollo/client
bundle (to pave the way for futuregql
optimizations)gql
from@apollo/client