Call onCompleted with data from cache (#2190)

* Call onCompleted with data from cache * Trigger `onCompleted` / `onError` callbacks after render Adjusts `<Query />` `onCompleted` and `onError` callbacks to be triggered via the `componentDidUpdate` lifecycle method. This ensures these callbacks can be used when data is fetched over the network, and when data is fetched from the local store (previsouly these callbacks were only being triggered when data was fetched over the network).

Author: olistic

Changelog.md

@@ -53,6 +53,12 @@
 - Typescript: use `Partial<TData>` instead of `TData | {}`, for the
  `QueryResult` `data` property. <br/>
  [@tgriesser](https://github.com/tgriesser) in [#2313](https://github.com/apollographql/react-apollo/pull/2313)
+- Adjust `<Query />` `onCompleted` and `onError` callbacks to be triggered
+ via the `componentDidUpdate` lifecycle method. This ensures these callbacks
+ can be used when data is fetched over the network, and when data is
+ fetched from the local store (previsouly these callbacks were only being
+ triggered when data was fetched over the network).
+ [@olistic](https://github.com/olistic) in [#2190](https://github.com/apollographql/react-apollo/pull/2190)
 
 ## 2.1.11 (August 9, 2018)
 

src/Query.tsx

@@ -166,6 +166,7 @@ export default class Query<TData = any, TVariables = OperationVariables> extends
  componentDidMount() {
  this.hasMounted = true;
  if (this.props.skip) return;
+
  this.startQuerySubscription();
  if (this.refetcherQueue) {
  const { args, resolve, reject } = this.refetcherQueue;
@@ -210,6 +211,19 @@ export default class Query<TData = any, TVariables = OperationVariables> extends
  this.hasMounted = false;
  }
 
+ componentDidUpdate() {
+ const { onCompleted, onError } = this.props;
+ if (onCompleted || onError) {
+ const currentResult = this.queryObservable!.currentResult();
+ const { loading, error, data } = currentResult;
+ if (onCompleted && !loading && !error) {
+ onCompleted(data);
+ } else if (onError && !loading && error) {
+ onError(error);
+ }
+ }
+ }
+
  render() {
  const { children } = this.props;
  const queryResult = this.getQueryResult();
@@ -321,17 +335,6 @@ export default class Query<TData = any, TVariables = OperationVariables> extends
  }
 
  private updateCurrentData = () => {
- const { onCompleted, onError } = this.props;
- if (onCompleted || onError) {
- const currentResult = this.queryObservable!.currentResult();
- const { loading, error, data } = currentResult;
- if (onCompleted && !loading && !error) {
- onCompleted(data);
- } else if (onError && !loading && error) {
- onError(error);
- }
- }
-
  // force a rerender that goes through shouldComponentUpdate
  if (this.hasMounted) this.forceUpdate();
  };

test/client/Query.test.tsx

@@ -713,25 +713,82 @@ describe('Query component', () => {
  });
 
  it('onCompleted with data', done => {
+ const query = gql`
+ query people($first: Int) {
+ allPeople(first: $first) {
+ people {
+ name
+ }
+ }
+ }
+ `;
+
+ const data1 = { allPeople: { people: [{ name: 'Luke Skywalker' }] } };
+ const data2 = { allPeople: { people: [{ name: 'Han Solo' }] } };
  const mocks = [
  {
- request: { query: allPeopleQuery },
- result: { data: allPeopleData },
+ request: { query, variables: { first: 1 } },
+ result: { data: data1 },
+ },
+ {
+ request: { query, variables: { first: 2 } },
+ result: { data: data2 },
  },
  ];
 
- const onCompleted = (queryData: Data | {}) => {
- expect(stripSymbols(queryData)).toEqual(allPeopleData);
- done();
- };
+ let count = 0;
 
- const Component = () => (
- <Query query={allPeopleQuery} onCompleted={onCompleted}>
- {() => {
- return null;
- }}
- </Query>
- );
+ class Component extends React.Component {
+ state = {
+ variables: {
+ first: 1,
+ },
+ };
+
+ componentDidMount() {
+ setTimeout(() => {
+ this.setState({
+ variables: {
+ first: 2,
+ },
+ });
+ setTimeout(() => {
+ this.setState({
+ variables: {
+ first: 1,
+ },
+ });
+ }, 50);
+ }, 50);
+ }
+
+ // Make sure `onCompleted` is called both when new data is being
+ // fetched over the network, and when data is coming back from
+ // the cache.
+ onCompleted(data: Data | {}) {
+ if (count === 0) {
+ expect(stripSymbols(data)).toEqual(data1);
+ }
+ if (count === 1) {
+ expect(stripSymbols(data)).toEqual(data2);
+ }
+ if (count === 2) {
+ expect(stripSymbols(data)).toEqual(data1);
+ done();
+ }
+ count += 1;
+ }
+
+ render() {
+ const { variables } = this.state;
+
+ return (
+ <AllPeopleQuery query={query} variables={variables} onCompleted={this.onCompleted}>
+ {() => null}
+ </AllPeopleQuery>
+ );
+ }
+ }
 
  wrapper = mount(
  <MockedProvider mocks={mocks} addTypename={false}>