feat(core/presentation): Reduce unnecessary renders in useLatestPromi…

…se, add tests

app/scripts/modules/core/src/presentation/hooks/useLatestPromise.hook.spec.tsx

@@ -0,0 +1,161 @@
+import * as React from 'react';
+import { mount } from 'enzyme';
+import { IUseLatestPromiseResult, useLatestPromise } from './useLatestPromise.hook';
+
+describe('useLatestPromise hook', () => {
+  // Remove the the refresh function for .isEqual assertions
+  function promiseState(call: IUseLatestPromiseResult<any>) {
+    const { refresh, ...state } = call;
+    return state;
+  }
+
+  function Component(props: any) {
+    const { promiseFactory, deps, onChange } = props;
+    const useLatestPromiseResult: IUseLatestPromiseResult<any> = useLatestPromise(promiseFactory, deps);
+    const { status, result, error, requestId } = useLatestPromiseResult;
+
+    React.useEffect(() => onChange(useLatestPromiseResult), [status, result, error, requestId]);
+    return <></>;
+  }
+
+  function defer() {
+    let resolve: Function, reject: Function;
+    const promise = new Promise((_resolve, _reject) => {
+      resolve = _resolve;
+      reject = _reject;
+    });
+    return { promise, resolve, reject };
+  }
+
+  it('has status NONE if no promise has been returned', () => {
+    const spy = jasmine.createSpy('onChange');
+    mount(<Component promiseFactory={() => null as any} deps={[]} onChange={spy} />);
+    expect(spy).toHaveBeenCalledTimes(1);
+
+    const result: IUseLatestPromiseResult<any> = spy.calls.mostRecent().args[0];
+    expect(promiseState(result)).toEqual({ status: 'NONE', result: undefined, error: undefined, requestId: 0 });
+  });
+
+  it('has status PENDING if a promise has been returned but has not yet resolved', () => {
+    const spy = jasmine.createSpy('onChange');
+    const deferred = defer();
+    mount(<Component promiseFactory={() => deferred.promise} deps={[]} onChange={spy} />);
+    expect(spy).toHaveBeenCalledTimes(2);
+
+    const result: IUseLatestPromiseResult<any> = spy.calls.mostRecent().args[0];
+    expect(promiseState(result)).toEqual({ status: 'PENDING', result: undefined, error: undefined, requestId: 0 });
+  });
+
+  it('has status RESOLVED if a promise resolved', async done => {
+    const spy = jasmine.createSpy('onChange');
+    const deferred = defer();
+    const component = mount(<Component promiseFactory={() => deferred.promise} deps={[]} onChange={spy} />);
+    expect(spy).toHaveBeenCalledTimes(2);
+
+    deferred.resolve('payload');
+    await deferred.promise;
+    component.setProps({});
+
+    expect(spy).toHaveBeenCalledTimes(3);
+    const result: IUseLatestPromiseResult<any> = spy.calls.mostRecent().args[0];
+    expect(promiseState(result)).toEqual({ status: 'RESOLVED', result: 'payload', error: undefined, requestId: 0 });
+    done();
+  });
+
+  it('has status REJECTED if a promise rejected', async done => {
+    const spy = jasmine.createSpy('onChange');
+    const deferred = defer();
+    const component = mount(<Component promiseFactory={() => deferred.promise} deps={[]} onChange={spy} />);
+    expect(spy).toHaveBeenCalledTimes(2);
+
+    deferred.reject('error');
+    try {
+      await deferred.promise;
+    } catch (error) {}
+    component.setProps({});
+
+    expect(spy).toHaveBeenCalledTimes(3);
+    const result: IUseLatestPromiseResult<any> = spy.calls.mostRecent().args[0];
+    expect(promiseState(result)).toEqual({ status: 'REJECTED', result: undefined, error: 'error', requestId: 0 });
+    done();
+  });
+
+  it('only handles the latest promise when multiple promises are pending', async done => {
+    const spy = jasmine.createSpy('onChange');
+    const deferred1 = defer();
+    const component = mount(<Component promiseFactory={() => deferred1.promise} deps={[1]} onChange={spy} />);
+    expect(spy).toHaveBeenCalledTimes(2);
+
+    const deferred2 = defer();
+    component.setProps({ promiseFactory: () => deferred2.promise, deps: [2] });
+    component.setProps({});
+    expect(spy).toHaveBeenCalledTimes(3);
+    expect(spy.calls.mostRecent().args[0].status).toEqual('PENDING');
+
+    deferred1.resolve('payload1');
+    await deferred1.promise;
+    component.setProps({});
+
+    expect(spy).toHaveBeenCalledTimes(3);
+    expect(spy.calls.mostRecent().args[0].status).toEqual('PENDING');
+
+    deferred2.resolve('payload2');
+    await deferred2.promise;
+    component.setProps({});
+
+    expect(spy).toHaveBeenCalledTimes(4);
+    const result: IUseLatestPromiseResult<any> = spy.calls.mostRecent().args[0];
+    expect(promiseState(result)).toEqual({ status: 'RESOLVED', result: 'payload2', error: undefined, requestId: 1 });
+    done();
+  });
+
+  it('gets a new promise if refresh() is called', async done => {
+    const spy = jasmine.createSpy('onChange');
+    const deferred = defer();
+    const promiseFactorySpy = jasmine.createSpy('promiseFactory').and.callFake(() => deferred.promise);
+    const component = mount(<Component promiseFactory={promiseFactorySpy} deps={[]} onChange={spy} />);
+    expect(promiseFactorySpy).toHaveBeenCalledTimes(1);
+
+    // initial promise is resolved.
+    deferred.resolve('payload');
+    await deferred.promise;
+    component.setProps({});
+
+    spy.calls.mostRecent().args[0].refresh();
+    component.setProps({});
+    expect(promiseFactorySpy).toHaveBeenCalledTimes(2);
+
+    done();
+  });
+
+  it('ignores old pending results if a newer promise is being processed', async done => {
+    const spy = jasmine.createSpy('onChange');
+    const deferred1 = defer();
+    const deferred2 = defer();
+    const component = mount(<Component promiseFactory={() => deferred1.promise} deps={[1]} onChange={spy} />);
+
+    component.setProps({ promiseFactory: () => deferred2.promise, deps: [2] });
+
+    // The first promise is resolved.
+    deferred1.resolve('payload1');
+    await deferred1.promise;
+    component.setProps({});
+
+    // The second promise is resolved.
+    deferred2.resolve('payload2');
+    await deferred2.promise;
+    component.setProps({});
+
+    expect(spy).toHaveBeenCalledTimes(4);
+    const allCalls = spy.calls.allArgs().map(args => promiseState(args[0]));
+    expect(allCalls[0]).toEqual({ status: 'NONE', result: undefined, error: undefined, requestId: 0 });
+    // initial request
+    expect(allCalls[1]).toEqual({ status: 'PENDING', result: undefined, error: undefined, requestId: 0 });
+    // second request
+    expect(allCalls[2]).toEqual({ status: 'PENDING', result: undefined, error: undefined, requestId: 1 });
+    // resolved second request
+    expect(allCalls[3]).toEqual({ status: 'RESOLVED', result: 'payload2', error: undefined, requestId: 1 });
+
+    done();
+  });
+});

app/scripts/modules/core/src/presentation/hooks/useLatestPromise.hook.ts

@@ -1,5 +1,6 @@
 import { IPromise } from 'angular';
 import { DependencyList, useEffect, useRef, useState } from 'react';
+import { useIsMountedRef } from './useIsMountedRef.hook';
 
 export type IRequestStatus = 'NONE' | 'PENDING' | 'REJECTED' | 'RESOLVED';
 
@@ -16,71 +17,78 @@ export interface IUseLatestPromiseResult<T> {
   requestId: number;
 }
 
+type IPromiseState<T> = Pick<IUseLatestPromiseResult<T>, 'result' | 'status' | 'error' | 'requestId'>;
+const initialPromiseState: IPromiseState<any> = {
+  error: undefined,
+  requestId: 0,
+  result: undefined,
+  status: 'NONE',
+};
+
 /**
  * A react hook which invokes a callback that returns a promise.
  * If multiple requests are made concurrently, only returns data from the latest request.
  *
  * This can be useful when fetching data based on a users keyboard input, for example.
  * This behavior is similar to RxJS switchMap.
  *
+ * example:
+ * const fetch = useLatestPromise(() => fetch(url + '?foo=" + foo).then(x=>x.json()), [foo]);
+ * return (fetch.status === 'RESOLVED' ? <pre>{JSON.stringify(fetch.result, null, 2)}</pre> :
+ *         fetch.status === 'REJECTED' ? <span>Error: {fetch.error}</span> :
+ *         fetch.status === 'PENDING' ? <span>Loading...</span> : null);
+ *
  * @param callback a callback that returns an IPromise
  * @param deps array of dependencies, which (when changed) cause the callback to be invoked again
  * @returns an object with the result and current status of the promise
  */
 export function useLatestPromise<T>(callback: () => IPromise<T>, deps: DependencyList): IUseLatestPromiseResult<T> {
-  const isMounted = useRef(false);
+  const isMountedRef = useIsMountedRef();
+  // Capture the isMountedRef.current value before effects run
+  const isInitialRender = !isMountedRef.current;
   const requestInFlight = useRef<IPromise<T>>();
-  const [status, setStatus] = useState<IRequestStatus>('NONE');
-  const [result, setResult] = useState<T>();
-  const [error, setError] = useState<any>();
-  const [requestId, setRequestId] = useState(0);
+  // A counter that is used to trigger the promise handling useEffect
+  const [requestIdTrigger, setRequestIdTrigger] = useState(0);
+  const [promiseState, setPromiseState] = useState<IPromiseState<T>>(initialPromiseState);
+  const { result, error, status, requestId } = promiseState;
 
   // Starts a new request (runs the callback again)
-  const refresh = () => setRequestId(id => id + 1);
+  const refresh = () => setRequestIdTrigger(id => id + 1);
 
   // refresh whenever any dependency in the dependency list changes
   useEffect(() => {
-    if (isMounted.current) {
-      refresh();
-    }
+    !isInitialRender && refresh();
   }, deps);
 
-  // Manage the mount/unmounted state
-  useEffect(() => {
-    isMounted.current = true;
-    return () => (isMounted.current = false);
-  }, []);
-
   // Invokes the callback and manages its lifecycle.
   // This is triggered when the requestId changes
   useEffect(() => {
     const promise = callback();
-    const isCurrent = () => isMounted.current === true && promise === requestInFlight.current;
+    const isCurrent = () => isMountedRef.current && promise === requestInFlight.current;
 
     // If no promise is returned from the callback, noop this effect.
     if (!promise) {
       return;
     }
 
-    setStatus('PENDING');
+    // Don't clear out previous error/result when a new request is pending
+    setPromiseState({ status: 'PENDING', error, result, requestId: requestIdTrigger });
     requestInFlight.current = promise;
 
     const resolve = (newResult: T) => {
       if (isCurrent()) {
-        setResult(newResult);
-        setStatus('RESOLVED');
+        setPromiseState({ status: 'RESOLVED', result: newResult, error: undefined, requestId: requestIdTrigger });
       }
     };
 
     const reject = (rejection: any) => {
       if (isCurrent()) {
-        setError(rejection);
-        setStatus('REJECTED');
+        setPromiseState({ status: 'REJECTED', result: undefined, error: rejection, requestId: requestIdTrigger });
       }
     };
 
     promise.then(resolve, reject);
-  }, [requestId]);
+  }, [requestIdTrigger]);
 
   return { result, status, error, refresh, requestId };
 }