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

Migrate React Apollo HOC/Components into Apollo Client #6558

Merged
merged 18 commits into from
Jul 8, 2020

Conversation

hwillson
Copy link
Member

@hwillson hwillson commented Jul 8, 2020

This PR migrates the React Apollo HOC/components functionality from https://github.com/apollographql/react-apollo into the Apollo Client codebase, making it available through the following new entry points:

import { Query, Mutation, Subscription } from '@apollo/client/react/components';
import { graphql } from '@apollo/client/react/hoc';

While we recommend using React hooks with Apollo Client (and our hooks are available from the main @apollo/client bundle directly), providing access to the HOC/Component based alternatives requires very little additional maintenance overhead. This is due to the fact that the HOC/Components call Apollo hooks behind the scenes (they're really just hook wrappers). Making this functionality available from Apollo Client directly also means that we can fully deprecate/archive the React Apollo project.

One more additional benefit to moving the HOC/Components into Apollo Client, is that we get to leverage their extensive test suite to help verify Apollo hooks functionality. The HOC/Component tests are validating a large number of edge cases that we are not validating through our hooks test suite directly.

It's important to note that HOC/Component functionality is not available from the main @apollo/client bundle directly, making this functionality fully tree-shakable by modern bundlers.

package.json Show resolved Hide resolved
@benjamn benjamn self-requested a review July 8, 2020 15:10
@hwillson
Copy link
Member Author

hwillson commented Jul 8, 2020

@benjamn @jcreighton I know it looks like there is a lot to review here, but I think it's safe to skip over the incoming tests. I updated most of the tests to work with the AC codebase a bit better (e.g. by using itAsync), but the overall functionality is still the same as it was in the React Apollo project. If the tests pass that should be good enough. And speaking of passing tests ... they're passing locally but not via CI, so I'm currently investigating. I'll let you know when things are good to go here. Thanks!

@hwillson hwillson requested a review from jcreighton July 8, 2020 15:31
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.

Love to see a bunch of new tests reliably running and passing (locally at least)!

Any idea why the bundle size decreased by 0.12kB? Maybe including prop-types in the external list in rollup.config.js?

package.json Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@hwillson hwillson force-pushed the migrate-hoc-components branch 2 times, most recently from 969d15c to 3d045dd Compare July 8, 2020 19:32
Move the `MockedProvider` tests from the `react-apollo` repo,
updating them to use hooks instead of Apollo HOC/Components.
Since we're shutting down the React Apollo repo, let's move the
Query, Mutation and Subscription components into this repo. At
some point in the future we're going to officially remove these
components, but since they already wrap our hooks internally,
there is very little additional overhead needed to maintain them
for now. These components will not be accessible from the main
`@apollo/client` entry point. They will be given their own entry
point in a future commit, making them accessible via
`@apollo/client/react/components` (which means modern bundlers
will be able to properly tree-shake them, when they aren't
being used).
This test was originally in the components `Query.test.tsx` file,
but for some reason it isn't working properly after the
component code was migrated into this repo. Instead of
spending more time troubleshooting it for the component code,
it's more important to verify it works with `useQuery` (which it
does).
Migrating the React Apollo `Mutation` component code into this repo
uncovered a bug with the way we're setting options in `useMutation`.
This commit makes sure we're no longer accidentally overwriting
`defaultOptions` settings with `undefined`.
The `fetchMore` test was triggering a React `act` error due to
the `result.fetchMore` call happening during render.
This commit makes RA's `graphql` (and related) HOC functionality
part of the Apollo Client project. All tests have been updated
to work with the AC codebase.
Having to make `hoist-non-react-statics` a dependency just for
the `graphql` HOC is unfortunate. This might change.
This commit creates `@apollo/client/react/components` and
`@apollo/client/react/hoc` package entry points.
We're running into an interesting Circle Workflows issue, where old
cached data is not being properly removed. As a troubleshooting
start, we'll bump the cache key version to make sure we're using a
new cache. As a bonus using v3 here lines up with AC v3.
Node 12 is the current LTS version.
We have a lot of delays intertwined in our old React Apollo tests
that aren't necessary (and actually lead to random test failures).
The delays in this test in particular cause CI to randomly fail.
Since we don't need these delays, let's get rid of them.
The previous iteration of this test failed randomly in CI.
Delays in the HOC SSR tests are causing random CI failures, so
let's get rid of all of them (they aren't needed to properly
verify functionality).
Adjustments to fix the use of `react-scripts` in the webpack
based app, as well as package lock updates.
@dsilvasc
Copy link

@apollo/react-hoc users who land on apollographql/react-apollo#3959 might benefit from a pointer to this migration.

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