Merge pull request #8596 from apollographql/brian-murder-querydata

React refactorings
apollographql · Aug 18, 2021 · a4dfee5 · a4dfee5
2 parents 92a7bed + f3e1f35
commit a4dfee5
Show file tree

Hide file tree

Showing 40 changed files with 2,811 additions and 2,794 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,18 @@
+## Apollo Client 3.5 (unreleased)
+
+### Bug Fixes
+- `useQuery` and `useLazyQuery` will now have observableQuery methods defined consistently. <br/> [@brainkim](https://github.com/brainkim) in [#8596](https://github.com/apollographql/apollo-client/pull/8596)
+
+- Calling `useLazyQuery` methods like `startPolling` will start the query <br/>[@brainkim](https://github.com/brainkim) in [#8596](https://github.com/apollographql/apollo-client/pull/8596)
+
+- Calling the `useLazyQuery` execution function will now behave more like `refetch`. `previousData` will be preserved. <br/>[@brainkim](https://github.com/brainkim) in [#8596](https://github.com/apollographql/apollo-client/pull/8596)
+
+- `standby` fetchPolicies will now act like `skip: true` more consistently <br/>[@brainkim](https://github.com/brainkim) in [#8596](https://github.com/apollographql/apollo-client/pull/8596)
+
+- Calling `refetch` on a skipped query will have no effect (https://github.com/apollographql/apollo-client/issues/8270). <br/>[@brainkim](https://github.com/brainkim) in [#8596](https://github.com/apollographql/apollo-client/pull/8596)
+
+- Improvements to `onError` and `onCompleted` functions, preventing them from firing continuously, and working with polling <br/>[@brainkim](https://github.com/brainkim) in [#8596](https://github.com/apollographql/apollo-client/pull/8596)
+
 ## Apollo Client 3.4.8
 
 ### Bug Fixes

diff --git a/config/entryPoints.js b/config/entryPoints.js
@@ -17,7 +17,6 @@ const entryPoints = [
   { dirs: ['react'] },
   { dirs: ['react', 'components'] },
   { dirs: ['react', 'context'] },
-  { dirs: ['react', 'data'] },
   { dirs: ['react', 'hoc'] },
   { dirs: ['react', 'hooks'] },
   { dirs: ['react', 'parser'] },

diff --git a/package.json b/package.json
@@ -41,7 +41,7 @@
     "resolve": "ts-node-script config/resolveModuleIds.ts",
     "clean": "rimraf -r dist coverage lib temp",
     "test": "jest --config ./config/jest.config.js",
-    "test:debug": "BABEL_ENV=server node --inspect-brk node_modules/.bin/jest --config ./config/jest.config.js --runInBand",
+    "test:debug": "BABEL_ENV=server node --inspect-brk node_modules/.bin/jest --config ./config/jest.config.js --runInBand --testTimeout 99999",
     "test:ci": "npm run test:coverage && npm run test:memory",
     "test:watch": "jest --config ./config/jest.config.js --watch",
     "test:memory": "cd scripts/memory && npm i && npm test",

diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap
@@ -256,15 +256,6 @@ Array [
 ]
 `;
 
-exports[`exports of public entry points @apollo/client/react/data 1`] = `
-Array [
-  "MutationData",
-  "OperationData",
-  "QueryData",
-  "SubscriptionData",
-]
-`;
-
 exports[`exports of public entry points @apollo/client/react/hoc 1`] = `
 Array [
   "graphql",
@@ -291,6 +282,7 @@ Array [
   "DocumentType",
   "operationName",
   "parser",
+  "verifyDocumentType",
 ]
 `;
 

diff --git a/src/__tests__/exports.ts b/src/__tests__/exports.ts
@@ -16,7 +16,6 @@ import * as linkWS from "../link/ws";
 import * as react from "../react";
 import * as reactComponents from "../react/components";
 import * as reactContext from "../react/context";
-import * as reactData from "../react/data";
 import * as reactHOC from "../react/hoc";
 import * as reactHooks from "../react/hooks";
 import * as reactParser from "../react/parser";
@@ -56,7 +55,6 @@ describe('exports of public entry points', () => {
   check("@apollo/client/react", react);
   check("@apollo/client/react/components", reactComponents);
   check("@apollo/client/react/context", reactContext);
-  check("@apollo/client/react/data", reactData);
   check("@apollo/client/react/hoc", reactHOC);
   check("@apollo/client/react/hooks", reactHooks);
   check("@apollo/client/react/parser", reactParser);

diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts
@@ -305,9 +305,11 @@ export class ObservableQuery<
     // (no-cache, network-only, or cache-and-network), override it with
     // network-only to force the refetch for this fetchQuery call.
     const { fetchPolicy } = this.options;
-    if (fetchPolicy === 'no-cache') {
+    if (fetchPolicy === 'standby' || fetchPolicy === 'cache-and-network') {
+      reobserveOptions.fetchPolicy = fetchPolicy;
+    } else if (fetchPolicy === 'no-cache') {
       reobserveOptions.fetchPolicy = 'no-cache';
-    } else if (fetchPolicy !== 'cache-and-network') {
+    } else {
       reobserveOptions.fetchPolicy = 'network-only';
     }
 

diff --git a/src/react/components/Query.tsx b/src/react/components/Query.tsx
@@ -9,7 +9,7 @@ export function Query<TData = any, TVariables = OperationVariables>(
 ) {
   const { children, query, ...options } = props;
   const result = useQuery(query, options);
-  return result ? children(result) : null;
+  return result ? children(result as any) : null;
 }
 
 export interface Query<TData, TVariables> {

diff --git a/src/react/components/__tests__/client/Mutation.test.tsx b/src/react/components/__tests__/client/Mutation.test.tsx
@@ -7,7 +7,12 @@ import { ApolloClient } from '../../../../core';
 import { ApolloError } from '../../../../errors';
 import { DataProxy, InMemoryCache as Cache } from '../../../../cache';
 import { ApolloProvider } from '../../../context';
-import { MockedProvider, MockLink, mockSingleLink } from '../../../../testing';
+import {
+  itAsync,
+  MockedProvider,
+  MockLink,
+  mockSingleLink,
+} from '../../../../testing';
 import { Query } from '../../Query';
 import { Mutation } from '../../Mutation';
 
@@ -156,24 +161,28 @@ describe('General Mutation testing', () => {
     expect(spy).toHaveBeenCalledWith(mocksProps[1].result);
   });
 
-  it('performs a mutation', async () => {
+  itAsync('performs a mutation', (resolve, reject) => {
     let count = 0;
     const Component = () => (
       <Mutation mutation={mutation}>
         {(createTodo: any, result: any) => {
-          if (count === 0) {
-            expect(result.loading).toEqual(false);
-            expect(result.called).toEqual(false);
-            createTodo();
-          } else if (count === 1) {
-            expect(result.called).toEqual(true);
-            expect(result.loading).toEqual(true);
-          } else if (count === 2) {
-            expect(result.called).toEqual(true);
-            expect(result.loading).toEqual(false);
-            expect(result.data).toEqual(data);
+          try {
+            if (count === 0) {
+              expect(result.loading).toEqual(false);
+              expect(result.called).toEqual(false);
+              createTodo();
+            } else if (count === 1) {
+              expect(result.called).toEqual(true);
+              expect(result.loading).toEqual(true);
+            } else if (count === 2) {
+              expect(result.called).toEqual(true);
+              expect(result.loading).toEqual(false);
+              expect(result.data).toEqual(data);
+            }
+            count++;
+          } catch (err) {
+            reject(err);
           }
-          count++;
           return <div />;
         }}
       </Mutation>
@@ -185,7 +194,7 @@ describe('General Mutation testing', () => {
       </MockedProvider>
     );
 
-    await wait();
+    wait().then(resolve, reject);
   });
 
   it('can bind only the mutation and not rerender by props', done => {
@@ -922,7 +931,7 @@ describe('General Mutation testing', () => {
     });
   });
 
-  it('allows a refetchQueries prop as string and variables have updated', async () => {
+  it('allows a refetchQueries prop as string and variables have updated', async () => new Promise((resolve, reject) => {
     const query = gql`
       query people($first: Int) {
         allPeople(first: $first) {
@@ -978,33 +987,42 @@ describe('General Mutation testing', () => {
           {(createTodo: any, resultMutation: any) => (
             <Query query={query} variables={variables}>
               {(resultQuery: any) => {
-                if (count === 0) {
-                  // "first: 1" loading
-                  expect(resultQuery.loading).toBe(true);
-                } else if (count === 1) {
-                  // "first: 1" loaded
-                  expect(resultQuery.loading).toBe(false);
-                  setTimeout(() => setVariables({ first: 2 }));
-                } else if (count === 2) {
-                  // "first: 2" loading
-                  expect(resultQuery.loading).toBe(true);
-                } else if (count === 3) {
-                  // "first: 2" loaded
-                  expect(resultQuery.loading).toBe(false);
-                  setTimeout(() => createTodo());
-                } else if (count === 4) {
-                  // mutation loading
-                  expect(resultMutation.loading).toBe(true);
-                } else if (count === 5) {
-                  // mutation loaded
-                  expect(resultMutation.loading).toBe(false);
-                } else if (count === 6) {
-                  // query refetched
-                  expect(resultQuery.loading).toBe(false);
-                  expect(resultMutation.loading).toBe(false);
-                  expect(resultQuery.data).toEqual(peopleData3);
+                try {
+                  if (count === 0) {
+                    // "first: 1" loading
+                    expect(resultQuery.loading).toBe(true);
+                  } else if (count === 1) {
+                    // "first: 1" loaded
+                    expect(resultQuery.loading).toBe(false);
+                    expect(resultQuery.data).toEqual(peopleData1);
+                    setTimeout(() => setVariables({ first: 2 }));
+                  } else if (count === 2) {
+                    expect(resultQuery.loading).toBe(false);
+                    expect(resultQuery.data).toEqual(peopleData1);
+                  } else if (count === 3) {
+                    // "first: 2" loading
+                    expect(resultQuery.loading).toBe(true);
+                  } else if (count === 4) {
+                    // "first: 2" loaded
+                    expect(resultQuery.loading).toBe(false);
+                    expect(resultQuery.data).toEqual(peopleData2);
+                    setTimeout(() => createTodo());
+                  } else if (count === 5) {
+                    // mutation loading
+                    expect(resultMutation.loading).toBe(true);
+                  } else if (count === 6) {
+                    // mutation loaded
+                    expect(resultMutation.loading).toBe(false);
+                  } else if (count === 7) {
+                    // query refetched
+                    expect(resultQuery.loading).toBe(false);
+                    expect(resultMutation.loading).toBe(false);
+                    expect(resultQuery.data).toEqual(peopleData3);
+                  }
+                  count++;
+                } catch (err) {
+                  reject(err);
                 }
-                count++;
                 return null;
               }}
             </Query>
@@ -1019,12 +1037,12 @@ describe('General Mutation testing', () => {
       </MockedProvider>
     );
 
-    await wait(() => {
-      expect(count).toBe(7);
-    });
-  });
+    wait(() => {
+      expect(count).toBe(8);
+    }).then(resolve, reject);
+  }));
 
-  it('allows refetchQueries to be passed to the mutate function', async () => {
+  it('allows refetchQueries to be passed to the mutate function', () => new Promise((resolve, reject) => {
     const query = gql`
       query getTodo {
         todo {
@@ -1071,18 +1089,22 @@ describe('General Mutation testing', () => {
         {(createTodo: any, resultMutation: any) => (
           <Query query={query}>
             {(resultQuery: any) => {
-              if (count === 0) {
-                setTimeout(() => createTodo({ refetchQueries }), 10);
-              } else if (count === 1) {
-                expect(resultMutation.loading).toBe(false);
-                expect(resultQuery.loading).toBe(false);
-              } else if (count === 2) {
-                expect(resultMutation.loading).toBe(true);
-                expect(resultQuery.data).toEqual(queryData);
-              } else if (count === 3) {
-                expect(resultMutation.loading).toBe(false);
+              try {
+                if (count === 0) {
+                  setTimeout(() => createTodo({ refetchQueries }), 10);
+                } else if (count === 1) {
+                  expect(resultMutation.loading).toBe(false);
+                  expect(resultQuery.loading).toBe(false);
+                } else if (count === 2) {
+                  expect(resultMutation.loading).toBe(true);
+                  expect(resultQuery.data).toEqual(queryData);
+                } else if (count === 3) {
+                  expect(resultMutation.loading).toBe(false);
+                }
+                count++;
+              } catch (err) {
+                reject(err);
               }
-              count++;
               return null;
             }}
           </Query>
@@ -1096,10 +1118,10 @@ describe('General Mutation testing', () => {
       </MockedProvider>
     );
 
-    await wait(() => {
+    wait(() => {
       expect(count).toBe(4);
-    });
-  });
+    }).then(resolve, reject);
+  }));
 
   it('has an update prop for updating the store after the mutation', async () => {
     const update = (_proxy: DataProxy, response: ExecutionResult) => {