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

Release 2.5.0 #4361

Merged
merged 88 commits into from Feb 26, 2019
Merged

Release 2.5.0 #4361

merged 88 commits into from Feb 26, 2019

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Jan 22, 2019

This PR tracks changes that are slated to be released in the next minor version of apollo-client.

PRs that introduce minor breaking changes should be merged into the release-2.5.0 branch instead of master, so that we can keep releasing patch updates in the meantime.

hwillson and others added 10 commits January 21, 2019 11:14
Includes updates to the `apollo-utilities` package to better
accommodate Apollo Client's local state handling capabilities.

- Adds a new transform `buildQueryFromSelectionSet' function to
  help construct a GraphQL Query from a Mutation selection set.
- Adds a new transform `removeClientSetsFromDocument` function
  to prune `@client` selection sets and fields from GraphQL
  documents'.
- Adds a new directive based `hasClientExports` function to see
  if a GraphQL document is using a `@client @export` directive
  combination.
- Adds a new `mergeDeep` utility function for deep cloning.
When using the `@client` directive, it might be desirable in
some cases to want to write a selection set to the store,
without having all of the selection set values available.
This is because the `@client` field values might have already
been written to the cache separately (e.g. via Apollo
Cache's `writeData` capabilities). Because of this, we'll
skip the missing field warning for fields with `@client`
directives.
This is a temporary change, while working on the local state
alpha changes.
This commit provides the bulk of the new Apollo Client local
state handling capabilities. It replaces the need to use
`apollo-link-state` in a link chain for local state management,
by merging local state handling directly into the Apollo Client
core. The majority of the new local state functionality can be
found in the `LocalState` class, which is tied into the
`QueryManager` to integrate with Queries, Mutations and
Subscriptions.

Key Changes:

- Replaces the need to use `apollo-link-state`.
- `apollo-link-state` defaults have been replaced with new
  initializer functions, which are a much more capable and
  flexible way to initialize the cache.
- Initializers and local resolvers can be set through the
  `ApolloClient` constructor, or set separately using new
  public API functions.
- A local schema can be set through `ApolloClient`, which
  can then be used by external tooling (like Apollo Client
  Devtools).
- Mixing remote and local (`@client` based) data together in
  queries is supported.
- Queries can now use the `@export` directive to export the
  result of a local resolver into a variable, that can then
  be automatically passed into a subsequent query. E.g.
  `@client @export(as: "someVar")`
- Full SSR support.
- Numerous bug fixes made to address the oustanding
  `apollo-link-state` issue backlog.
Various tests to cover Apollo Client local state handling.
Remove Boost's dependency on `apollo-link-state`, and wire it up
with AC's local state functionality.
By default, local resolvers are controlled by a query's
`fetchPolicy`. If a query that has configured local
resolvers is run with a fresh cache, and that query is
using `ApolloClient.query`'s default `fetchPolicy` of
`cache-and-network`, the local resolvers will be fired
and the result will be stored in the AC cache. If the
same query fires again however, since the query result
already exists in the cache, it will be loaded and used
instead of firing the query's local resolvers again. This
functionality can be altered by using a `fetchPolicy` of
`no-cache` for example, but setting a `fetchPolicy` impacts
an entire query. So if mixing local and remote results and
using a `fetchPolicy` of `no-cache`, local resolvers will
always run, but so will the fetch to retrieve network
based data.

To address this, AC's local state functionality includes
a new `resolverPolicy` approach. Setting a `resolverPolicy`
of `resolver-always` makes sure local resolvers are
always fired for a query, on every request. While this
approach works, it is inflexible due to the fact that
it does not provide a way to only always run certain local
resolvers associated with a query, instead of running all
of them. Apollo Client's `fetchPolicy` approach has also
historically demonstrated that getting `fetchPolicy` settings
right can be a bit tricky (especially for newcomers to the
Apollo ecosystem), so adding another configurable policy
based approach is not overly desirable.

The changes in this commit remove the `resolverPolicy`
functionality. They then ensure that fields marked with
`@client(always: true)` always have their local resolvers
run, on each request. This provides a way to control
exactly which parts of a query should have its local
resolvers always run, and which parts can continue to
leverage the cache.

Technical side note: when using `@client(always: true)`,
the full query will be resolved from the cache first,
before any local resolvers are run. So if data already
exists in the cache for a field that's marked with
`@client(always: true)`, it's loaded first as part of
reading the fully executed query from the cache (including
local and remote results). That data (if it exists) is
then overwritten by the result from the associated local
resolver, and returned as part of the query result. This
load then override approach makes sure that the integrity
of the cache is not affected by running local resolvers.
The QueryManager#stop method cancels all pending fetches by running
this.fetchQueryRejectFns. However, this leads to some unhandled promise
rejections during tests, which might go unnoticed because they don't cause
the test suite to fail.

This commit makes the QueryManager#stop method a bit more aggressive about
stopping active queries and unsubscribing from observables, which prevents
the unhandled rejections.
…oroughly

Prevent unhandled rejections by stopping QueryManager more thoroughly.
@benjamn benjamn added this to the Release 2.5.0 milestone Jan 22, 2019
benjamn and others added 3 commits January 22, 2019 18:46
This reverts commit 9739ff6.

Now that we have a uniform interface for terminating ApolloClient
instances (#4336), there should be no need for any external code to access
the QueryScheduler abstraction, which this commit removes.

We should wait to merge and release this change until after
apollographql/react-apollo#2741 has been merged
and released, so that we don't break older versions of MockedProvider.
…removal

Un-revert "Improve (and shorten) query polling implementation. (#4243)"
@danilobuerger
Copy link
Contributor

@benjamn Whats the release schedule on this? I would like to get 2 PRs in there as well with minor breaking changes.

@benjamn
Copy link
Member Author

benjamn commented Jan 23, 2019

@danilobuerger Do you have the authority to add those PRs to the milestone? https://github.com/apollographql/apollo-client/milestone/15

If not, just let me know which ones you have in mind!

@danilobuerger
Copy link
Contributor

@benjamn Thanks! That worked. For when is the released planned so I get my PRs ready in time?

@benjamn
Copy link
Member Author

benjamn commented Jan 23, 2019

The tallest tent-post is #4338, which I would say needs at least another week of work/testing. Internally, though, our sprint goals are more about finishing the local state work so our own teams can dogfood it, and less about the final 2.5.0 release, so I think we're pretty flexible about when we merge this PR. Within the next two weeks, let's say?

@danilobuerger
Copy link
Contributor

Alright, perfect.

@benjamn benjamn force-pushed the release-2.5.0 branch 2 times, most recently from aa8c7fa to e399ad8 Compare January 24, 2019 14:58
 - apollo-boost@0.3.0-rc.0
 - apollo-cache-inmemory@1.5.0-rc.0
 - apollo-cache@1.2.0-rc.0
 - apollo-client@2.5.0-rc.0
 - graphql-anywhere@4.2.0-rc.0
 - apollo-boost@0.3.0-rc.1
 - apollo-cache-inmemory@1.5.0-rc.1
 - apollo-cache@1.2.0-rc.1
 - apollo-client@2.5.0-rc.1
 - apollo-utilities@1.2.0-rc.1
 - graphql-anywhere@4.2.0-rc.1
`ObservableQuery.getCurrentResult` can now return a result that
includes a `stale` property, which wasn't accounted for in the
returned `ApolloCurrentQueryResult` type.
The current code is returning a "Spread types may only be
created from object types" error when trying to understand
`{ ...prev, ...updater(prev) }`. This commit makes sure
typescript can tell the result is an object.
This reverts commit 2f197f8.

This change isn't necessary when using typescript >= 3.2, which
Apollo Client is configured to use. My global typescript was
stuck as 3.1, hence the unnecessary commit.
Recent jest config changes inadvertantly disabled all local
state tests.
Changes to make sure calling `npm run watch` in any of this
repo's child packages ensures the final bundles are built
after compilation has completed. For the full details on
why this is needed, see:

apollographql/react-apollo#2765
In an effort to simulate `defaults` behaviour from
`apollo-link-state`, we're leveraging cache results when
running local resolvers. The idea being that if a local resolver
isn't defined for a `@client` field, the local resolver
handling code can then fallback on using any matching cache
results, to resolve the field. While this works in theory,
it has introduced a few problems, like the one reported in #4474.

Since local resolvers adhere to Apollo Client's query fetch
policy, by defalut the cache is consulted first, when trying
to resolve a `@client` field. This means we shouldn't need to
attempt to resolve from the cache again, when processing local
resolvers, in most cases. There are a few situations where we
might want to do this, but the requirements are theoretical at
this point, and can be addressed in future changes if needed.

This commit removes the extra cache check, and adds
a test to verify that the behaviour reported in #4474 is fixed.
 - apollo-boost@0.3.0-rc.2
 - apollo-cache-inmemory@1.5.0-rc.2
 - apollo-cache@1.2.0-rc.2
 - apollo-client@2.5.0-rc.2
 - apollo-utilities@1.2.0-rc.2
 - graphql-anywhere@4.2.0-rc.2
@hwillson
Copy link
Member

Hi all - the Apollo Client 2.5.0 release candidate is now ready for testing. If you're interested in trying it out, update to apollo-client@next (and optionally react-apollo@next). Thanks!

benjamn and others added 8 commits February 25, 2019 13:33
The only reason we currently store/manage client typeDefs in the Apollo
Client codebase is so that they can be consumed by the Apollo DevTools in
development.

Because the DevTools do not have a public API, it's important for
developers to continue passing any client typeDefs to the ApolloClient
constructor, but we do not have to expose those typeDefs through any
convenient public API, and the LocalState class should not need to know
about them at all, much less perform any expensive normalization, since
all of that work can be done by the DevTools.

Instead, the typeDefs are now exposed as client.typeDefs, exactly as they
were originally passed to the ApolloClient constructor. Local resolvers
will no longer receive context.schemas, but that's no great loss because
the LocalState implementation is new in apollo-client@2.5.0. It would be
much harder to remove that functionality after shipping v2.5.0, which is
why it's important to do it now.

Thanks to this change, we're back to having zero imports of
graphql/language/printer in the Apollo Client codebase (though the printer
is still used by apollo-link).

@hwillson @justinanastos I realize there's been some churn in the way the
DevTools and the client communicate recently, but the good news is we can
iterate freely because it's a private API. Let me know if you have any
questions about this change!
* Enable local state only when client resolvers provided.

If an application was previously using apollo-link-state, updating to
apollo-client@2.5.0 could cause problems because @client fields are now
stripped by the integrated LocalState API, and thus will not be passed
into the link chain.

This commit should ease the transition by enabling the LocalState
functionality only if client resolvers were passed to the ApolloClient
constructor, or the LocalState#setResolvers method has been called.

If no client resolvers have been specified, @client fields will remain in
the query passed to the link chain, so apollo-link-state can still process
them, though a warning will be logged in development.

If you want to use @client directives to read from or write to the cache
without running resolver functions, you can pass an empty resolvers:{} map
to enable the LocalState functionality (including the stripping of @client
fields from queries).

* Refine private LocalState resolvers field type.

The setResolvers method normalizes Resolvers[] arrays into a combined
(non-array) Resolvers object.
 - apollo-boost@0.3.0-rc.3
 - apollo-client@2.5.0-rc.3
@hwillson hwillson merged commit 26c4ff3 into master Feb 26, 2019
@hwillson hwillson deleted the release-2.5.0 branch February 26, 2019 20:34
@benjamn benjamn restored the release-2.5.0 branch March 15, 2019 15:46
@benjamn
Copy link
Member Author

benjamn commented Mar 15, 2019

@hwillson Just a note that we should probably keep release-x.y.0 branches around, in case we need to backport any bug fixes.

@hwillson
Copy link
Member

Oops - great point @benjamn, will do!

@benjamn benjamn mentioned this pull request Mar 16, 2019
@benjamn benjamn mentioned this pull request Aug 1, 2019
31 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants