diff --git a/Changelog.md b/Changelog.md index 6119192bcc..dc23f05056 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,16 @@ # Change log +## 3.1.0 (TBD) + +### Improvements + +- Change the default query `data` state from `{}` to `undefined`. + + > **NOTE: THIS IS A POTENTIALLY BREAKING CHANGE.** We're still working out the best way to roll this out. + + This change aligns all parts of the React Apollo query cycle so that `data` is always `undefined` if there is no data, instead of `data` being converted into an empty object. This change impacts the initial query response, initial SSR response, `data` value when errors occur, `data` value when skipping, etc. All of these areas are now aligned to only ever return a value for `data` if there really is a value to return (instead of making it seem like there is one by converting to `{}`).
+ [@hwillson](https://github.com/hwillson) in [#3388](https://github.com/apollographql/react-apollo/pull/3388) + ## 3.0.1 (2019-08-15) ### Improvements diff --git a/packages/components/src/__tests__/client/Query.test.tsx b/packages/components/src/__tests__/client/Query.test.tsx index 7578e653e5..0415876291 100644 --- a/packages/components/src/__tests__/client/Query.test.tsx +++ b/packages/components/src/__tests__/client/Query.test.tsx @@ -9,7 +9,7 @@ import { } from '@apollo/react-testing'; import { DocumentNode } from 'graphql'; import gql from 'graphql-tag'; -import { render, cleanup } from '@testing-library/react'; +import { render, cleanup, wait } from '@testing-library/react'; import { ApolloLink } from 'apollo-link'; import { Query } from '@apollo/react-components'; @@ -954,7 +954,7 @@ describe('Query component', () => { ); }); - it('onError with error', done => { + it('onError with error', async () => { const error = new Error('error occurred'); const mockError = [ { @@ -965,7 +965,6 @@ describe('Query component', () => { const onErrorFunc = (queryError: ApolloError) => { expect(queryError.networkError).toEqual(error); - done(); }; const Component = () => ( @@ -981,6 +980,8 @@ describe('Query component', () => { ); + + await wait(); }); }); @@ -1958,7 +1959,7 @@ describe('Query component', () => { {(result: any) => { const { data, loading } = result; if (!loading) { - expect(data).toEqual({}); + expect(data).toBeUndefined(); done(); } return null; @@ -2133,7 +2134,7 @@ describe('Query component', () => { {({ data }: any) => { - expect(data).toEqual({}); + expect(data).toBeUndefined(); return null; }} diff --git a/packages/components/src/__tests__/client/__snapshots__/Query.test.tsx.snap b/packages/components/src/__tests__/client/__snapshots__/Query.test.tsx.snap index c1bd72cd89..44acb4cc10 100644 --- a/packages/components/src/__tests__/client/__snapshots__/Query.test.tsx.snap +++ b/packages/components/src/__tests__/client/__snapshots__/Query.test.tsx.snap @@ -21,7 +21,7 @@ Object { exports[`Query component calls the children prop: result in render prop while loading 1`] = ` Object { "called": true, - "data": Object {}, + "data": undefined, "error": undefined, "fetchMore": [Function], "loading": true, diff --git a/packages/hooks/src/__tests__/useQuery.test.tsx b/packages/hooks/src/__tests__/useQuery.test.tsx index 62e626ddd9..320fa8795a 100644 --- a/packages/hooks/src/__tests__/useQuery.test.tsx +++ b/packages/hooks/src/__tests__/useQuery.test.tsx @@ -58,6 +58,25 @@ describe('useQuery Hook', () => { ); }); + + it('should keep data as undefined until data is actually returned', done => { + const Component = () => { + const { data, loading } = useQuery(CAR_QUERY); + if (loading) { + expect(data).toBeUndefined(); + } else { + expect(data).toEqual(CAR_RESULT_DATA); + done(); + } + return null; + }; + + render( + + + + ); + }); }); describe('Polling', () => { diff --git a/packages/hooks/src/data/QueryData.ts b/packages/hooks/src/data/QueryData.ts index 2b353d4dce..6e8593ec27 100644 --- a/packages/hooks/src/data/QueryData.ts +++ b/packages/hooks/src/data/QueryData.ts @@ -173,7 +173,7 @@ export class QueryData extends OperationData { loading: true, networkStatus: NetworkStatus.loading, called: true, - data: {} + data: undefined }; if (this.context && this.context.renderPromises) { @@ -275,7 +275,7 @@ export class QueryData extends OperationData { this.previousData.result && this.previousData.result.loading === loading && this.previousData.result.networkStatus === networkStatus && - isEqual(this.previousData.result.data, data || {}) + isEqual(this.previousData.result.data, data) ) { return; } @@ -314,15 +314,9 @@ export class QueryData extends OperationData { } private getQueryResult(): QueryResult { - let result = { - data: Object.create(null) as TData - } as any; - - // Attach bound methods - Object.assign( - result, - this.observableQueryFields(this.currentObservable.query!) - ); + let result: any = { + ...this.observableQueryFields(this.currentObservable.query!) + }; // When skipping a query (ie. we're not querying for data but still want // to render children), make sure the `data` is cleared out and @@ -340,7 +334,6 @@ export class QueryData extends OperationData { const currentResult = this.currentObservable.query!.getCurrentResult(); const { loading, partial, networkStatus, errors } = currentResult; let { error, data } = currentResult; - data = data || (Object.create(null) as TData); // Until a set naming convention for networkError and graphQLErrors is // decided upon, we map errors (graphQLErrors) to the error options. @@ -348,13 +341,24 @@ export class QueryData extends OperationData { error = new ApolloError({ graphQLErrors: errors }); } - Object.assign(result, { loading, networkStatus, error, called: true }); + result = { + ...result, + loading, + networkStatus, + error, + called: true + }; if (loading) { - const previousData = this.previousData.result - ? this.previousData.result.data - : {}; - Object.assign(result.data, previousData, data); + const previousData = + this.previousData.result && this.previousData.result.data; + result.data = + previousData && data + ? { + ...previousData, + ...data + } + : previousData || data; } else if (error) { Object.assign(result, { data: (this.currentObservable.query!.getLastResult() || ({} as any)) @@ -365,14 +369,14 @@ export class QueryData extends OperationData { const { partialRefetch } = this.getOptions(); if ( partialRefetch && - Object.keys(data).length === 0 && + !data && partial && fetchPolicy !== 'cache-only' ) { // When a `Query` component is mounted, and a mutation is executed // that returns the same ID as the mounted `Query`, but has less // fields in its result, Apollo Client's `QueryManager` returns the - // data as an empty Object since a hit can't be found in the cache. + // data as `undefined` since a hit can't be found in the cache. // This can lead to application errors when the UI elements rendered by // the original `Query` component are expecting certain data values to // exist, and they're all of a sudden stripped away. To help avoid @@ -385,7 +389,7 @@ export class QueryData extends OperationData { return result; } - Object.assign(result.data, data); + result.data = data; } } @@ -417,7 +421,7 @@ export class QueryData extends OperationData { } if (onCompleted && !error) { - onCompleted(data as TData); + onCompleted(data); } else if (onError && error) { onError(error); } diff --git a/packages/ssr/src/__tests__/useQuery.test.tsx b/packages/ssr/src/__tests__/useQuery.test.tsx index b07647e251..9bed5ce203 100644 --- a/packages/ssr/src/__tests__/useQuery.test.tsx +++ b/packages/ssr/src/__tests__/useQuery.test.tsx @@ -62,11 +62,11 @@ describe('useQuery Hook SSR', () => { }); }); - it('should initialize data as an empty object when loading', () => { + it('should initialize data as `undefined` when loading', () => { const Component = () => { const { data, loading } = useQuery(CAR_QUERY); if (loading) { - expect(data).toEqual({}); + expect(data).toBeUndefined(); } return null; };