Repository: FormidableLabs/urql
Pull request #78 (2018-05-11 15:26:18)

Refactor client to emit events on subscribers
The observable "events" on the client are a little hard to follow since they're not self-descriptive. Instead we can make them into actual "actions"/"events", let them have a typed event, and make them more type-safe.


Patch:
diff --git a/src/components/client.tsx b/src/components/client.tsx
index 62e7eb88..6d1f063f 100644
--- a/src/components/client.tsx
+++ b/src/components/client.tsx
@@ -1,14 +1,17 @@
 import { Component, ReactNode } from 'react';
+import { CombinedError } from '../modules/error';
+import { hashString } from '../modules/hash';
+import { formatTypeNames } from '../modules/typenames';
+import { zipObservables } from '../utils/zip-observables';
+
 import {
+  ClientEvent,
+  ClientEventType,
   IClient,
   IExchangeResult,
   IMutation,
   IQuery,
 } from '../interfaces/index';
-import { CombinedError } from '../modules/error';
-import { hashString } from '../modules/hash';
-import { formatTypeNames } from '../modules/typenames';
-import { zipObservables } from '../utils/zip-observables';
 
 export interface IClientProps {
   client: IClient; // Client instance
@@ -171,30 +174,37 @@ export default class UrqlClient extends Component<IClientProps, IClientState> {
     }
   };
 
-  update = (changedTypes, response, refresh) => {
-    if (refresh === true) {
+  update = (event: ClientEvent) => {
+    const { type } = event;
+
+    if (type === ClientEventType.RefreshAll) {
+      // RefreshAll indicates that the component should refetch its queries
       this.fetch();
-    }
-    let invalidated = false;
-    if (this.props.shouldInvalidate) {
-      invalidated = this.props.shouldInvalidate(
-        changedTypes,
-        this.typeNames,
-        response,
-        this.state.data
-      );
-    } else if (this.props.typeInvalidation !== false) {
-      // Check connection typenames, derived from query, for presence of mutated typenames
-      this.typeNames.forEach(typeName => {
-        if (changedTypes.indexOf(typeName) !== -1) {
-          invalidated = true;
-        }
-      });
-    }
-    // If it has any of the type names that changed
-    if (invalidated) {
-      // Refetch the data from the server
-      this.fetch({ skipCache: true });
+      return;
+    } else if (type === ClientEventType.InvalidateTypenames) {
+      // InvalidateTypenames instructs us to reevaluate this component's typenames
+      const { typenames, changes } = event.payload;
+
+      let invalidated = false;
+      if (this.props.shouldInvalidate) {
+        invalidated = this.props.shouldInvalidate(
+          typenames,
+          this.typeNames,
+          changes,
+          this.state.data
+        );
+      } else if (this.props.typeInvalidation !== false) {
+        // Check connection typenames, derived from query, for presence of mutated typenames
+        invalidated = this.typeNames.some(
+          typeName => typenames.indexOf(typeName) !== -1
+        );
+      }
+
+      // If it has any of the type names that changed
+      if (invalidated) {
+        // Refetch the data from the server
+        this.fetch({ skipCache: true });
+      }
     }
   };
 
diff --git a/src/interfaces/client.ts b/src/interfaces/client.ts
index 89d82ea6..dd853dc1 100644
--- a/src/interfaces/client.ts
+++ b/src/interfaces/client.ts
@@ -1,6 +1,7 @@
 import Observable from 'zen-observable-ts';
 
 import { ICache } from './cache';
+import { ClientEvent } from './events';
 import { IExchangeResult } from './exchange';
 import { IQuery } from './query';
 
@@ -18,7 +19,5 @@ export interface IClient {
   executeMutation$(mutationObject: IQuery): Observable<IExchangeResult['data']>;
   executeMutation(mutationObject: IQuery): Promise<IExchangeResult['data']>;
   refreshAllFromCache(): void;
-  subscribe(
-    callback: (changedTypes: string[], reponse: object) => void
-  ): () => void;
+  subscribe(callback: (event: ClientEvent) => void): () => void;
 }
diff --git a/src/interfaces/events.ts b/src/interfaces/events.ts
new file mode 100644
index 00000000..9f241f22
--- /dev/null
+++ b/src/interfaces/events.ts
@@ -0,0 +1,17 @@
+import { IExchangeResult } from './exchange';
+
+export enum ClientEventType {
+  InvalidateTypenames = 'InvalidateTypenames',
+  RefreshAll = 'RefreshAll',
+}
+
+export interface IClientEvent<E, P> {
+  payload: P;
+  type: E;
+}
+
+export type ClientEvent = IClientEvent<
+  ClientEventType.InvalidateTypenames,
+  { typenames: string[]; changes: IExchangeResult }
+> &
+  IClientEvent<ClientEventType.RefreshAll, void>;
diff --git a/src/interfaces/index.ts b/src/interfaces/index.ts
index 4595ed56..1fd8648d 100644
--- a/src/interfaces/index.ts
+++ b/src/interfaces/index.ts
@@ -5,5 +5,6 @@ export { IClientOptions } from './client-options';
 export { ICache } from './cache';
 export { IExchange, IExchangeResult } from './exchange';
 export { IGraphQLError } from './error';
+export { ClientEventType, ClientEvent } from './events';
 export { IOperation } from './operation';
 export { ISubscriptionObserver, ISubscription } from './subscription';
diff --git a/src/modules/client.ts b/src/modules/client.ts
index 28cbf909..c9ba151a 100644
--- a/src/modules/client.ts
+++ b/src/modules/client.ts
@@ -1,6 +1,8 @@
 import Observable from 'zen-observable-ts';
 
 import {
+  ClientEvent,
+  ClientEventType,
   ICache,
   IClientOptions,
   IExchange,
@@ -17,11 +19,11 @@ import { httpExchange } from './http-exchange';
 export default class Client {
   url: string;
   store: object; // Internal store
-  subscriptions: object; // Map of subscribed Connect components
   subscriptionSize: number; // Used to generate IDs for subscriptions
   cache: ICache; // Cache object
   exchange: IExchange; // Exchange to communicate with GraphQL APIs
   fetchOptions: RequestInit | (() => RequestInit); // Options for fetch call
+  subscriptions: { [id: string]: (ClientEvent) => void }; // Map of subscribed Connect components
 
   constructor(opts?: IClientOptions) {
     if (!opts) {
@@ -49,17 +51,19 @@ export default class Client {
     this.refreshAllFromCache = this.refreshAllFromCache.bind(this);
   }
 
-  updateSubscribers(typenames, changes) {
+  updateSubscribers(typenames: string[], changes: IExchangeResult) {
     // On mutation, call subscribed callbacks with eligible typenames
+
     /* tslint:disable-next-line forin */
     for (const sub in this.subscriptions) {
-      this.subscriptions[sub](typenames, changes);
+      this.subscriptions[sub]({
+        payload: { typenames, changes },
+        type: ClientEventType.InvalidateTypenames,
+      });
     }
   }
 
-  subscribe(
-    callback: (changedTypes: string[], response: object) => void
-  ): () => void {
+  subscribe(callback: (event: ClientEvent) => void): () => void {
     // Create an identifier, add callback to subscriptions
     const id = this.subscriptionSize++;
     this.subscriptions[id] = callback;
@@ -75,8 +79,9 @@ export default class Client {
     return new Promise(resolve => {
       /* tslint:disable-next-line forin */
       for (const sub in this.subscriptions) {
-        this.subscriptions[sub](null, null, true);
+        this.subscriptions[sub]({ type: ClientEventType.RefreshAll });
       }
+
       resolve();
     });
   }


Test patch:
diff --git a/src/tests/components/client.test.tsx b/src/tests/components/client.test.tsx
index a1c8cfba..6872dc4e 100644
--- a/src/tests/components/client.test.tsx
+++ b/src/tests/components/client.test.tsx
@@ -7,6 +7,7 @@ import { hashString } from '../../modules/hash';
 import { formatTypeNames } from '../../modules/typenames';
 import { default as ClientModule } from '../../modules/client';
 import { subscriptionExchange } from '../../modules/subscription-exchange';
+import { ClientEventType } from '../../interfaces/index';
 import renderer from 'react-test-renderer';
 
 describe('Client Component', () => {
@@ -524,7 +525,9 @@ describe('Client Component', () => {
     // NOTE: Delay here waits for the fetch to flush and complete, since
     // dedupExchange would deduplicate it otherwise
     setTimeout(() => {
-      client.getInstance().update(null, null, true);
+      client.getInstance().update({
+        type: ClientEventType.RefreshAll,
+      });
 
       setTimeout(() => {
         expect(spy).toHaveBeenCalledTimes(2);
diff --git a/src/tests/modules/client.test.ts b/src/tests/modules/client.test.ts
index 00ba9982..0bca0fc1 100644
--- a/src/tests/modules/client.test.ts
+++ b/src/tests/modules/client.test.ts
@@ -1,5 +1,6 @@
 import Client from '../../modules/client';
 import { defaultCache } from '../../modules/default-cache';
+import { ClientEventType } from '../../interfaces/events';
 
 describe('Client', () => {
   beforeEach(() => {
@@ -10,6 +11,7 @@ describe('Client', () => {
 
   it('should throw without options provided', () => {
     expect(() => {
+      /* tslint:disable-next-line no-unused-expression */
       new Client();
     }).toThrowError('Please provide configuration object');
   });
@@ -17,7 +19,7 @@ describe('Client', () => {
   it('should throw without a url provided', () => {
     expect(() => {
       // @ts-ignore
-      new Client({});
+      new Client({}); /* tslint:disable-line no-unused-expression */
     }).toThrowError('Please provide a URL for your GraphQL API');
   });
 
@@ -110,8 +112,14 @@ describe('Client', () => {
       client.subscribe(spy);
       const typenames = ['a', 'b'];
       const changes = { a: 5 };
+
+      const event = {
+        payload: { typenames, changes },
+        type: ClientEventType.InvalidateTypenames,
+      };
+
       client.updateSubscribers(typenames, changes);
-      expect(spy).toBeCalledWith(typenames, changes);
+      expect(spy).toBeCalledWith(event);
     });
   });
 
@@ -128,7 +136,9 @@ describe('Client', () => {
       const spy = jest.fn();
       client.subscribe(spy);
       client.refreshAllFromCache();
-      expect(spy).toBeCalledWith(null, null, true);
+
+      const event = { type: ClientEventType.RefreshAll };
+      expect(spy).toBeCalledWith(event);
     });
   });