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

Allow cache.{read,write}Query to take options.id. #5930

Merged
merged 3 commits into from
Feb 11, 2020

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Feb 10, 2020

Writing data to a specific entity object in the cache should not require the additional boilerplate of creating a named fragment with a type condition.

All the cache really cares about is the top-level selection set of the query or fragment used to read or write data, and from that perspective queries are an almost perfect substitute for fragments, in almost all cases.

In other words, this code

cache.writeFragment({
  id,
  fragment: gql`
    fragment UnnecessaryFragmentName on UnnecessaryType {
      firstName
      lastName
    }
  `,
  data: {
    __typename: "UnnecessaryType",
    firstName,
    lastName,
  },
});

can now be written as just

cache.writeQuery({
  id, // This option is new!
  query: gql`{ firstName lastName }`,
  data: { firstName, lastName },
});

Once you get used to using queries instead of fragments, you may begin to wonder why you ever thought fragments were a good idea. To save you some existential turmoil: fragments are still a good idea if you really need their type condition behavior (that is, if you want the fragment to match only when the on SomeType condition holds), but otherwise passing options.id and options.query to cache.readQuery or cache.writeQuery is a lot simpler and just as powerful.

If you need further convincing, the cache.readFragment and cache.writeFragment methods actually convert the given options.fragment document to a query document (using getFragmentQueryDocument) which includes the given options.fragment as a ...field. You can skip that needless conversion by just providing options.query to readQuery or writeQuery, and reserving readFragment and writeFragment for the rare cases where fragments actually have advantages over queries.

We are not removing the cache.readFragment and cache.writeFragment methods at this time, though we may consider deprecating them in the future.

As a side benefit, these changes happen to solve our longest outstanding feature request, apollographql/apollo-feature-requests#1, which recently inspired #5866. That's because options.rootId is now always defined when we do this check.

Writing data to a specific entity object should not require the additional
boilerplate of creating a named fragment with a type condition. All the
cache really cares about is the top-level selection set of the query or
fragment used to read or write data, and from that perspective queries are
an almost perfect substitute for fragments, in almost all cases.

In other words, this code

  cache.writeFragment({
    id,
    fragment: gql`
      fragment UnnecessaryFragmentName on UnnecessaryType {
        firstName
        lastName
      }
    `,
    data: {
      __typename: "UnnecessaryType",
      firstName,
      lastName,
    },
  });

can now be written as just

  cache.writeQuery({
    id,
    query: gql`{ firstName lastName }`,
    data: { firstName, lastName },
  });

Once you get used to using queries instead of fragments, you may begin to
wonder why you ever thought fragments were a good idea. To save you some
existential turmoil: fragments are still a good idea if you really need
their type condition behavior (that is, if you want the fragment to match
only when the `on SomeType` condition holds), but otherwise passing
options.id and options.query to cache.readQuery or cache.writeQuery is a
lot simpler and just as powerful.

If you need further convincing, the cache.readFragment and
cache.writeFragment methods actually convert the given options.fragment
document to a query document (using getFragmentQueryDocument) which
includes the given options.fragment as a ...field. You can skip that
needless conversion by just providing options.query to readQuery or
writeQuery, and reserving readFragment and writeFragment for the rare
cases where fragments actually have advantages over queries.

We are not removing the cache.readFragment and cache.writeFragment methods
at this time, though we may consider deprecating them in the future.

As a side benefit, these changes happen to solve our longest outstanding
feature request, apollographql/apollo-feature-requests#1,
which recently inspired #5866.
That's because options.rootId is now always defined when we do this check:
https://github.com/apollographql/apollo-client/blob/9cd5e8f7552db65437b5e59b605268a336977e45/src/cache/inmemory/inMemoryCache.ts#L130-L134
@benjamn benjamn added this to the Release 3.0 milestone Feb 10, 2020
@benjamn benjamn self-assigned this Feb 10, 2020
@benjamn benjamn changed the title Allow readQuery and writeQuery to take options.id. Allow cache.{read,write}Query to take options.id. Feb 10, 2020
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

I ❤️ this @benjamn - looks great, thanks!

@codecov

This comment has been minimized.

@benjamn benjamn merged commit 6a4e460 into master Feb 11, 2020
@benjamn benjamn deleted the allow-readQuery-and-writeQuery-to-take-options.id branch February 11, 2020 16:53
benjamn added a commit that referenced this pull request Sep 29, 2020
Closes apollographql/apollo-feature-requests#1,
by making cache.read no longer throw under any circumstances. This is a
long-standing feature request that I partially addressed in #5930, but
this commit goes all the way.

Since the current behavior (sometimes returning T, sometimes null, and
sometimes throwing) has been in place for so long, we do not make this
change lightly, and we should state precisely what is changing: in every
case where cache.read would previously throw an exception, it will now
return null instead.

Since these null results have the effect of hiding which fields were
missing, you may wish to switch from cache.readQuery to cache.diff to gain
more insight into why cache.read returned null. In fact, cache.diff (along
with cache.watch) are the primary ApolloCache methods that Apollo Client
uses internally, because the cache.diff API provides so much more useful
information than cache.read provides.

If you would prefer for cache.read to return partial data rather than
null, you can now pass returnPartialData:true to cache.readQuery and
cache.readFragment, though the default must remain false instead of true,
for the reasons explained in the code comments.

In the positive column, null should be easier to handle in update
functions that use the readQuery/writeQuery pattern for updating the
cache. Not only can you avoid wrapping cache.readQuery with a try-catch
block, but you can safely spread ...null into an object literal, which is
often something that happens between readQuery and writeQuery.

If you were relying on the exception-throwing behavior, and you have
application code that is not prepared to handle null, please leave a
comment describing your use case. We will let these changes bake in AC3.3
betas until we are confident they have been adequately tested.
@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.

None yet

2 participants