diff --git a/Changelog.md b/Changelog.md index 7451c6f18b..655e8daa0b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,10 @@ - Fix an infinite loop caused by using `setState` in the `onError` / `onCompleted` callbacks of the `Query` component.
[@chenesan](https://github.com/chenesan) in [#2751](https://github.com/apollographql/react-apollo/pull/2751) +- Fixed an issue that prevented good results from showing up in a `Query` + component, after an error was received, variables were adjusted, and then + the good data was fetched.
+ [@MerzDaniel](https://github.com/MerzDaniel) in [#2718](https://github.com/apollographql/react-apollo/pull/2718) ### Improvements diff --git a/src/Query.tsx b/src/Query.tsx index 367219919b..42280f7948 100644 --- a/src/Query.tsx +++ b/src/Query.tsx @@ -345,10 +345,15 @@ export default class Query extends this.updateCurrentData(); }, error: error => { - this.resubscribeToQuery(); - // Quick fix for https://github.com/apollostack/react-apollo/issues/378 + 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(); + } if (!error.hasOwnProperty('graphQLErrors')) throw error; - this.updateCurrentData(); }, }); @@ -365,13 +370,15 @@ export default class Query extends private resubscribeToQuery() { this.removeQuerySubscription(); + // Unfortunately, if `lastError` is set in the current + // `queryObservable` when the subscription is re-created, + // the subscription will immediately receive the error, which will + // cause it to terminate again. To avoid this, we first clear + // the last error/result from the `queryObservable` before re-starting + // the subscription, and restore it afterwards (so the subscription + // has a chance to stay open). const lastError = this.queryObservable!.getLastError(); - const lastResult = this.lastResult; - - // If lastError is set, the observable will immediately - // send it, causing the stream to terminate on initialization. - // We clear everything here and restore it afterward to - // make sure the new subscription sticks. + const lastResult = this.queryObservable!.getLastResult(); this.queryObservable!.resetLastResults(); this.startQuerySubscription(); Object.assign(this.queryObservable!, { lastError, lastResult }); diff --git a/test/client/Query.test.tsx b/test/client/Query.test.tsx index 92d26ac6b5..9e39a5e7e0 100644 --- a/test/client/Query.test.tsx +++ b/test/client/Query.test.tsx @@ -1387,6 +1387,107 @@ describe('Query component', () => { ); }); + it( + 'should not persist previous result errors when a subsequent valid ' + + 'result is received', + done => { + const query: DocumentNode = gql` + query somethingelse ($variable: Boolean) { + allPeople(first: 1, yetisArePeople: $variable) { + people { + name + } + } + }`; + + const data = { allPeople: { people: [{ name: 'Luke Skywalker' }] } }; + const variableGood = { variable: true } + const variableBad = { variable: false } + + const link = mockSingleLink( + { + request: { + query, + variables: variableGood, + }, + result: { + data, + }, + }, + { + request: { + query, + variables: variableBad, + }, + result: { + errors: [new Error('This is an error!')], + }, + }, + { + request: { + query, + variables: variableGood, + }, + result: { + data, + }, + }, + ); + + const client = new ApolloClient({ + link, + cache: new Cache({ addTypename: false }), + }); + + let count = 0; + const DummyComp = (props: any) => { + if (!props.loading) { + try { + switch (count++) { + case 0: + expect(props.data.allPeople).toBeTruthy(); + expect(props.error).toBeFalsy(); + // Change query variables to trigger bad result. + setTimeout(() => { + wrapper!.setProps({ variables: variableBad }); + }, 0); + break; + case 1: + // Error should be received, but last known good value + // should still be accessible (in-case the UI needs it). + expect(props.error).toBeTruthy(); + expect(props.data.allPeople).toBeTruthy(); + // Change query variables to trigger a good result. + setTimeout(() => { + wrapper!.setProps({ variables: variableGood }); + }, 0); + break + case 2: + // Good result should be received without any errors. + expect(props.error).toBeFalsy(); + expect(props.data.allPeople).toBeTruthy(); + done(); + break; + default: + done.fail('Unknown count'); + } + } catch (error) { + done.fail(error); + } + } + return null; + } + + wrapper = mount( + + {(result: any) => { + return ; + }} + + ); + } + ); + it('should not repeatedly call onCompleted if setState in it', done => { const query = gql` query people($first: Int) {