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

Prevent @client fields from being passed into the Link chain #5982

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

hwillson
Copy link
Member

Apollo Client 2.x allowed @client fields to be passed into the link chain if resolvers were not set in the constructor. This allowed @client fields to be passed into Links like apollo-link-state. Apollo Client 3 enforces that @client fields are local only, meaning they are no longer passed into the link chain, under any circumstances. We're making this change as we're slowly starting to deprecate the Local State API (and local resolvers) in favor of the new Type Policies API. Since we want people to be able to use the Type Policies API (and @client) without local resolvers, we don't want to force them to always define resolvers: {} in their ApolloClient constructor, to prevent @client fields from being passed into the link chain.

If anyone wants to preserve this behavior in AC 3, they can consider using their own directive (instead of @client), and using that directive in the link chain.

@hwillson hwillson force-pushed the remove-client-in-link-support branch from 84466b2 to 011b62e Compare February 23, 2020 11:11
@hwillson hwillson self-assigned this Feb 23, 2020
@hwillson hwillson added this to the Release 3.0 milestone Feb 23, 2020
@codecov
Copy link

codecov bot commented Feb 23, 2020

Codecov Report

Merging #5982 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5982      +/-   ##
==========================================
- Coverage   95.06%   95.05%   -0.01%     
==========================================
  Files          89       89              
  Lines        3705     3702       -3     
  Branches      911      879      -32     
==========================================
- Hits         3522     3519       -3     
  Misses        160      160              
  Partials       23       23              

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15536f5...7e2fa1c. Read the comment docs.

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 great! Can we add something to the migration guide about this?

@@ -35,6 +35,9 @@
- **[BREAKING?]** Remove `fixPolyfills.ts`, except when bundling for React Native. If you have trouble with `Map` or `Set` operations due to frozen key objects in React Native, either update React Native to version 0.59.0 (or 0.61.x, if possible) or investigate why `fixPolyfills.native.js` is not included in your bundle. <br/>
[@benjamn](https://github.com/benjamn) in [#5962](https://github.com/apollographql/apollo-client/pull/5962)

- **[BREAKING]** Apollo Client 2.x allowed `@client` fields to be passed into the `link` chain if `resolvers` were not set in the constructor. This allowed `@client` fields to be passed into Links like `apollo-link-state`. Apollo Client 3 enforces that `@client` fields are local only, meaning they are no longer passed into the `link` chain, under any circumstances. <br/>
Copy link
Member

Choose a reason for hiding this comment

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

Very clear and to the point!

@jeffreyffs
Copy link

I've got a question around this topic. Going forward, how do I get auto-mocked results for @client directives? Previously, I would use SchemaLink and addMockFunctionsToSchema, this became slightly more painful with AC2 and it seems like there is going to be no way to do this with AC3.

Switching to MockedProvider is painful and I hate trying to exactly match the request/result dance. Using a real ApolloProvider with a mocked schema is a WAY better DX... but I can't see how I'm going to get @client directive queries automatically mocked in AC3.

I'd love to see some documentation around this topic! Let me know if you'd like an issue opened for tracking or anything else.

PS: Love the work you guys are doing here ❤️

@dylanwulf
Copy link
Contributor

@jeffreyffs I totally agree that a real ApolloProvider with a mocked schema is way better DX. We use that for all our client side tests. In our case, we don't mock the @client fields -- we just let the resolvers take care of it. We don't have very many @client fields though, so your situation may be different from ours.

@hwillson
Copy link
Member Author

Thanks for the feedback @jeffreyffs @dylanwulf. We're making this change as we really want to enforce that @client fields are local only, and will never leave your applications local environment. We think the mental model surrounding @client / local state fields is easier to reason about if this is always the case. While the SchemaLink + addMockFunctionsToSchema approach works well, the fact that it works with @client fields is a side effect of how Apollo Link was originally implemented in Apollo Client, and is not something we want to support moving forward. Instead, we'd recommend leveraging Apollo Client 3's Type/Field Policies API for mocking. You can create mocked versions of your @client fields by defining field policies that return fake data, like:

const cache = new InMemoryCache({
  typePolicies: {
    Query: {
      fields: {
        isLoggedIn() {
          return faker.random.boolean();
        },
        user() {
          return {
            firstName: faker.name.firstName(),
            lastName: faker.name.lastName(),
            __typename: 'User'
          }
        }
      }
    }
  }
});

This is just a quick example, as there are definitely more sophisticated and automatic methods that could be developed by the community for this (e.g. a function that wraps all of your type policies and when called in NODE_ENV === 'testing' mode leverages your local typeDefs and a library like faker to auto-mock all the @client fields you want it to).

@hwillson hwillson force-pushed the remove-client-in-link-support branch from e53d34b to b93016a Compare February 25, 2020 15:44
@jeffreyffs
Copy link

@hwillson Thanks for the detailed response! Don't get me wrong, I'm all for simplifying local state management. The current implementation is probably my least favourite thing about Apollo Client.

I like that idea and I look forward to utilizing Type/field policies in the near future.

Apollo Client 2.x allowed `@client` fields to be passed into the
`link` chain if `resolvers` were not set in the constructor. This
allowed `@client` fields to be passed into Links like
`apollo-link-state`. Apollo Client 3 enforces that `@client` fields
are local only, meaning they are no longer passed into the `link`
chain, under any circumstances. We're making this change as we're
slowly starting to deprecate the Local State API (and local
resolvers) in favor of the new Type Policies API. Since we want
people to be able to use the Type Policies API (and `@client`)
without local resolvers, we don't want to force them to always
define `resolvers: {}` in their `ApolloClient` constructor, to
prevent `@client` fields from being passed into the `link` chain.

If anyone wants to preserve this behavior in AC 3, they can
consider using their own directive (instead of `@client`), and
using that directive in the `link` chain.
@hwillson hwillson force-pushed the remove-client-in-link-support branch from 3d5d395 to 7e2fa1c Compare February 26, 2020 18:55
@hwillson hwillson merged commit 86bfff8 into master Feb 26, 2020
@hwillson hwillson deleted the remove-client-in-link-support branch February 26, 2020 19:21
@JReinhold
Copy link

To continue on @freshollie point, this now means that when mocking @client queries in a MockedProvider, we need to use the resolvers prop instead of the mocks prop. This was a bit surprising to me when I read @freshollie comment, and I think it requires pretty deep knowledge about Apollo's internals to figure that out. At the very least, it should probably be mentioned in the "Testing React components" docs.

For anyone passing by, here's a simple example that should give you an idea on how to mock local queries:

const GET_POSTS = gql`
  query {
    GetPosts @client {
      id
    }
  }
`;

// broken - doesn't work because the query has the @client directive
const mock = {
  request: {
    query: GET_POSTS,
  },
  result: {
    data: {
      GetPosts: [{ id: 1 }],
    },
  },
};

render(
  <MockedProvider mocks={[mock]} addTypename={false}>
    <Posts />
  </MockedProvider>
);

// works as expected
const resolvers = {
  Query: {
    GetPosts: () => [{ id: 1 }],
  },
};

render(
  <MockedProvider resolvers={resolvers} addTypename={false}>
    <Posts />
  </MockedProvider>
);

@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.

6 participants