From a1e0b6d41158cfcf447dfe69474e79257d5c26b8 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Tue, 18 Jun 2019 06:39:42 -0400 Subject: [PATCH] Fix always loading issue caused by error handling (#3107) * Fix always loading issue caused by error handling The React Apollo `Query` component relies on Apollo Client's `ObservableQuery->getCurrentResult` method to get the results of a query to display. When `ObservableQuery` encounters an error, it stores that error in its associated query store, so that it can be passed back to React Apollo for handling. Unfortunately, the error stored in the query store isn't cleared out until a component unmounts, which means components that handle an error response followed by a valid response, won't get the chance to render the valid response. They're stuck rendering the error state, since the error stored in the `ObservableQuery` query store is never removed. This commit calls into a new `ObservableQuery.resetQueryStoreErrors` method, to clear out previously used errors, before moving on to handle subsequent responses. This means a component can handle an error, then receive and display a valid subsequent response. Fixes: https://github.com/apollographql/react-apollo/issues/3090 * Changelog update * Update deps to use new `apollo-client` 2.6.3 --- Changelog.md | 13 +++++++++---- package-lock.json | 6 +++--- package.json | 4 ++-- src/Query.tsx | 18 ++++++++++-------- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/Changelog.md b/Changelog.md index 38c642d6d2..f8aa35c8d4 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,15 +4,15 @@ ### Improvements -- Make sure `MockedProvider` is using the proper CJS/ESM bundle, when +- Make sure `MockedProvider` is using the proper CJS/ESM bundle, when referencing `ApolloProvider`.
[@jure](https://github.com/jure) in [#3029](https://github.com/apollographql/react-apollo/pull/3029). -- Adjust the `ApolloContext` definition to play a bit more nicely with +- Adjust the `ApolloContext` definition to play a bit more nicely with `React.createContext` types.
[@JoviDeCroock](https://github.com/JoviDeCroock) in [#3018](https://github.com/apollographql/react-apollo/pull/3018) -- The result of a mutation is now made available to the wrapped component, +- The result of a mutation is now made available to the wrapped component, when using the `graphql` HOC.
- [@andycarrell](https://github.com/andycarrell) in [#3008](https://github.com/apollographql/react-apollo/pull/3008) + [@andycarrell](https://github.com/andycarrell) in [#3008](https://github.com/apollographql/react-apollo/pull/3008) - Check equality of stringified variables in the `MockLink` to improve debugging experience used by `MockedProvider`.
[@evans](https://github.com/evans) in [#3078](https://github.com/apollographql/react-apollo/pull/3078) @@ -26,6 +26,11 @@ - Fix typescript error caused by `query` being mandatory in the `fetchMore` signature.
[@HsuTing](https://github.com/HsuTing) in [#3065](https://github.com/apollographql/react-apollo/pull/3065) +- Fixes an issue that caused the `Query` component to get stuck in an always + loading state, caused by receiving an error (meaning subsequent valid + responses couldn't be handled). The `Query` component can now handle an + error in a response, then continue to handle a valid response afterwards.
+ [@hwillson](https://github.com/hwillson) in [#3107](https://github.com/apollographql/react-apollo/pull/3107) ## 2.5.6 (2019-05-22) diff --git a/package-lock.json b/package-lock.json index 9ffa393d77..249ddd4987 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1332,9 +1332,9 @@ } }, "apollo-client": { - "version": "2.6.2", - "resolved": "https://registry.npmjs.org/apollo-client/-/apollo-client-2.6.2.tgz", - "integrity": "sha512-oks1MaT5x7gHcPeC8vPC1UzzsKaEIC0tye+jg72eMDt5OKc7BobStTeS/o2Ib3e0ii40nKxGBnMdl/Xa/p56Yg==", + "version": "2.6.3", + "resolved": "https://registry.npmjs.org/apollo-client/-/apollo-client-2.6.3.tgz", + "integrity": "sha512-DS8pmF5CGiiJ658dG+mDn8pmCMMQIljKJSTeMNHnFuDLV0uAPZoeaAwVFiAmB408Ujqt92oIZ/8yJJAwSIhd4A==", "dev": true, "requires": { "@types/zen-observable": "^0.8.0", diff --git a/package.json b/package.json index d1f2ecb200..884a46076c 100644 --- a/package.json +++ b/package.json @@ -100,7 +100,7 @@ "trailingComma": "all" }, "peerDependencies": { - "apollo-client": "^2.6.0", + "apollo-client": "^2.6.3", "react": "^15.0.0 || ^16.0.0", "react-dom": "^15.0.0 || ^16.0.0", "graphql": "^0.11.0 || ^0.12.0 || ^0.13.0 || ^14.0.0" @@ -124,7 +124,7 @@ "@types/zen-observable": "0.8.0", "apollo-cache": "1.3.2", "apollo-cache-inmemory": "1.6.2", - "apollo-client": "2.6.2", + "apollo-client": "2.6.3", "apollo-link": "1.2.11", "babel-core": "6.26.3", "babel-jest": "24.8.0", diff --git a/src/Query.tsx b/src/Query.tsx index 3e486fedce..2fd7642abb 100644 --- a/src/Query.tsx +++ b/src/Query.tsx @@ -349,14 +349,8 @@ export default class Query extends this.updateCurrentData(); }, error: error => { - if (!this.lastResult) { - // We only want to remove the old subscription, and start a new - // subscription, when an error was received and we don't have a - // previous result stored. This means either no previous result was - // received due to problems fetching data, or the previous result - // has been forcefully cleared out. - this.resubscribeToQuery(); - } + this.resubscribeToQuery(); + if (!error.hasOwnProperty('graphQLErrors')) throw error; this.updateCurrentData(); }, @@ -510,6 +504,14 @@ export default class Query extends }; } + // When the component is done rendering stored query errors, we'll + // remove those errors from the `ObservableQuery` query store, so they + // aren't re-displayed on subsequent (potentially error free) + // requests/responses. + setTimeout(() => { + this.queryObservable!.resetQueryStoreErrors(); + }); + data.client = this.client; return data; };