Fix memoization for frequency observables

packages/light.js/src/frequency/blocks.ts

@@ -5,7 +5,9 @@
 
 import BigNumber from 'bignumber.js';
 import { filter } from 'rxjs/operators';
+import * as memoizee from 'memoizee';
 
+import { createApiFromProvider, getApi } from '../api';
 import createPubsubObservable from './utils/createPubsubObservable';
 import { FrequencyObservableOptions } from '../types';
 
@@ -19,23 +21,25 @@ export function onEveryBlock$(options?: FrequencyObservableOptions) {
 }
 
 /**
- * Observable that emits on every 2nd block.
+ * Given an api object, return Observable that emits on every 2nd block.
+ * Pure function version of {@link onEvery2Blocks}.
  *
- * @param options - Options to pass to {@link FrequencyObservable}.
+ * @param api - The Api object.
  */
-export function onEvery2Blocks$(options?: FrequencyObservableOptions) {
-  return onEveryBlock$(options).pipe(
+const onEvery2BlocksWithApi$ = memoizee((api: any) =>
+  onEveryBlock$({ provider: api.provider }).pipe(
     filter(n => +n % 2 === 0) // Around ~30s on mainnet // TODO Use isEqualTo and mod from bignumber.js
-  );
-}
+  )
+);
 
 /**
- * Observable that emits on every 4th block.
+ * Observable that emits on every 2nd block.
  *
  * @param options - Options to pass to {@link FrequencyObservable}.
  */
-export function onEvery4Blocks$(options?: FrequencyObservableOptions) {
-  return onEveryBlock$(options).pipe(
-    filter(n => +n % 4 === 0) // Around ~1min on mainnet // TODO Use isEqualTo and mod from bignumber.js
-  );
+export function onEvery2Blocks$(options: FrequencyObservableOptions = {}) {
+  const { provider } = options;
+  const api = provider ? createApiFromProvider(provider) : getApi();
+
+  return onEvery2BlocksWithApi$(api);
 }

packages/light.js/src/frequency/frequency.spec.ts

@@ -8,7 +8,7 @@ import { take } from 'rxjs/operators';
 import frequency from './frequency';
 import { FrequencyObservable, FrequencyKey, FrequencyMap } from '../types';
 import isObservable from '../utils/isObservable';
-import { resolveApi } from '../utils/testHelpers/mockApi';
+import { rejectApi, resolveApi } from '../utils/testHelpers/mockApi';
 import { setApi } from '@parity/light.js/src/api';
 
 /**
@@ -22,7 +22,7 @@ const testFrequency = (
   resolveWith: any = 'foo'
 ) =>
   describe(`${name} frequency`, () => {
-    beforeAll(() => {
+    beforeEach(() => {
       setApi(resolveApi(resolveWith));
     });
 
@@ -34,11 +34,27 @@ const testFrequency = (
       expect(() => frequency$().subscribe()).not.toThrow();
     });
 
-    it('function should return the same Observable upon re-running (memoization)', () => {
+    it('should return the same Observable upon re-running (memoization)', () => {
       const initial$ = frequency$();
       expect(frequency$()).toBe(initial$);
     });
 
+    if (
+      [
+        'onAccountsChanged',
+        'onAccountsInfoChanged',
+        'onEveryBlock$',
+        'onEvery2Blocks$',
+        'onSyncingChanged'
+      ].includes(name)
+    ) {
+      it('should not return the same Observable if we change Api in between', () => {
+        const initial$ = frequency$();
+        setApi(rejectApi());
+        expect(frequency$()).not.toBe(initial$);
+      });
+    }
+
     it('should return values', done => {
       frequency$()
         .pipe(take(1))

packages/light.js/src/frequency/frequency.ts

@@ -6,16 +6,15 @@
 import * as accounts from './accounts';
 import * as blocks from './blocks';
 import * as health from './health';
-import { memoizeAll } from '../utils/memoizeAll';
 import * as other from './other';
 import * as time from './time';
 
-const frequency = memoizeAll({
+const frequency = {
   ...accounts,
   ...blocks,
   ...health,
   ...other,
   ...time
-});
+};
 
 export default frequency;

packages/light.js/src/frequency/other.ts

@@ -4,14 +4,19 @@
 // SPDX-License-Identifier: MIT
 
 import { of } from 'rxjs';
+import * as memoizee from 'memoizee';
 
-import { FrequencyObservable, FrequencyObservableOptions } from '../types';
+import { FrequencyObservableOptions } from '../types';
 
 /**
  * Observable that emits only once.
  *
  * @param options - Options to pass to {@link FrequencyObservable}.
  */
-export function onStartup$(_?: FrequencyObservableOptions) {
+function onStartup$(_?: FrequencyObservableOptions) {
   return of(0);
 }
+// @ts-ignore
+onStartup$ = memoizee(onStartup$);
+
+export { onStartup$ };

packages/light.js/src/frequency/time.ts

@@ -4,29 +4,35 @@
 // SPDX-License-Identifier: MIT
 
 import { timer } from 'rxjs';
+import * as memoizee from 'memoizee';
 
-import { FrequencyObservable, FrequencyObservableOptions } from '../types';
+import { FrequencyObservableOptions } from '../types';
 
 /**
  * Observable that emits on every second.
  */
-export const onEverySecond$: FrequencyObservable<number> = (
-  _?: FrequencyObservableOptions
-) => timer(0, 1000);
-onEverySecond$.metadata = { name: 'onEverySecond$' };
+function onEverySecond$(_?: FrequencyObservableOptions) {
+  return timer(0, 1000);
+}
+// @ts-ignore
+onEverySecond$ = memoizee(onEverySecond$);
 
 /**
  * Observable that emits on every other second.
  */
-export const onEvery2Seconds$: FrequencyObservable<number> = (
-  _?: FrequencyObservableOptions
-) => timer(0, 2000);
-onEvery2Seconds$.metadata = { name: 'onEvery2Seconds$' };
+function onEvery2Seconds$(_?: FrequencyObservableOptions) {
+  return timer(0, 2000);
+}
+// @ts-ignore
+onEvery2Seconds$ = memoizee(onEvery2Seconds$);
 
 /**
  * Observable that emits every five seconds.
  */
-export const onEvery5Seconds$: FrequencyObservable<number> = (
-  _?: FrequencyObservableOptions
-) => timer(0, 5000);
-onEvery5Seconds$.metadata = { name: 'onEvery5Seconds$' };
+function onEvery5Seconds$(_?: FrequencyObservableOptions) {
+  return timer(0, 5000);
+}
+// @ts-ignore
+onEvery5Seconds$ = memoizee(onEvery5Seconds$);
+
+export { onEverySecond$, onEvery2Seconds$, onEvery5Seconds$ };

packages/light.js/src/frequency/utils/createPubsubObservable.ts

@@ -5,14 +5,59 @@
 
 import * as debug from 'debug';
 import { FrequencyObservableOptions } from '../../types';
+import * as memoizee from 'memoizee';
 import { Observable, Observer, timer } from 'rxjs';
 import { switchMap } from 'rxjs/operators';
 
 import { createApiFromProvider, getApi } from '../../api';
 import { distinctReplayRefCount } from '../../utils/operators/distinctReplayRefCount';
 
 /**
- * Observable that emits on each pubsub event.
+ * Given an api, returns an Observable that emits on each pubsub event.
+ * Pure function version of {@link createPubsubObservable}.
+ *
+ * @ignore
+ */
+const createPubsubObservableWithApi = memoizee(
+  <T>(pubsub: string, api: any) => {
+    const [namespace, method] = pubsub.split('_');
+
+    // There's a chance the provider doesn't support pubsub, for example
+    // MetaMaskProvider. In this case, as suggested on their Github, the best
+    // solution for now is to poll.
+    if (!api.isPubSub) {
+      debug('@parity/light.js:api')(
+        `Pubsub not available for ${
+          api.provider ? api.provider.constructor.name : 'current Api'
+        } provider, polling "${pubsub}" every second.`
+      );
+
+      return timer(0, 1000).pipe(
+        switchMap(api[namespace][method])
+      ) as Observable<T>;
+    }
+
+    return Observable.create((observer: Observer<T>) => {
+      const subscription = api.pubsub[namespace][method](
+        (error: Error, result: any) => {
+          // TODO use @parity/api type for result
+          if (error) {
+            observer.error(error);
+          } else {
+            observer.next(result);
+          }
+        }
+      );
+      return () =>
+        subscription.then((subscriptionId: string) =>
+          api.pubsub.unsubscribe(subscriptionId)
+        );
+    }).pipe(distinctReplayRefCount()) as Observable<T>;
+  }
+);
+
+/**
+ * Given a provider, returns an Observable that emits on each pubsub event.
  *
  * @ignore
  * @example onAccountsChanged$, onEveryBlock$...
@@ -21,40 +66,9 @@ const createPubsubObservable = <T>(
   pubsub: string,
   { provider }: FrequencyObservableOptions = {}
 ) => {
-  const [namespace, method] = pubsub.split('_');
   const api = provider ? createApiFromProvider(provider) : getApi();
 
-  // There's a chance the provider doesn't support pubsub, for example
-  // MetaMaskProvider. In this case, as suggested on their Github, the best
-  // solution for now is to poll.
-  if (!api.isPubSub) {
-    debug('@parity/light.js:api')(
-      `Pubsub not available for ${
-        api.provider ? api.provider.constructor.name : 'current Api'
-      } provider, polling "${pubsub}" every second.`
-    );
-
-    return timer(0, 1000).pipe(
-      switchMap(() => api[namespace][method]())
-    ) as Observable<T>;
-  }
-
-  return Observable.create((observer: Observer<T>) => {
-    const subscription = api.pubsub[namespace][method](
-      (error: Error, result: any) => {
-        // TODO use @parity/api type for result
-        if (error) {
-          observer.error(error);
-        } else {
-          observer.next(result);
-        }
-      }
-    );
-    return () =>
-      subscription.then((subscriptionId: string) =>
-        api.pubsub.unsubscribe(subscriptionId)
-      );
-  }).pipe(distinctReplayRefCount()) as Observable<T>;
+  return createPubsubObservableWithApi<T>(pubsub, api);
 };
 
 export default createPubsubObservable;

packages/light.js/src/rpc/other/makeContract.ts

@@ -29,35 +29,37 @@ interface MakeContract {
  *
  * @param address - The contract address.
  * @param abiJson - The contract abi.
+ * @param api - The api Object.
  * @return - The contract object as defined in @parity/api.
  */
 const getContract = memoizee(
-  (address: Address, abiJson: any[], provider: any) => {
-    const api = provider ? createApiFromProvider(provider) : getApi();
-    return api.newContract(abiJson, address);
-  }, // use types from @parity/abi
+  (address: Address, abiJson: any[], api: any) =>
+    api.newContract(abiJson, address), // use types from @parity/abi
   { length: 1 } // Only memoize by address
 );
 
 /**
- * Create a contract.
+ * Create a contract, givan an api object.
+ * Pure function version of {@link makeContract}.
  *
+ * @ignore
  * @param address - The contract address.
- * @param - The contract abi.
+ * @param abiJson - The contract abi.
+ * @param api - The api Object.
  * @return - An object whose keys are all the functions of the
  * contract, and each function return an Observable which will fire when the
  * function resolves.
  */
-export const makeContract = memoizee(
-  (address: Address, abiJson: any[], options: { provider?: any } = {}) => {
-    const { provider } = options;
+const makeContractWithApi = memoizee(
+  (address: Address, abiJson: any[], api: any) => {
     const abi = new Abi(abiJson); // use types from @parity/abi
+
     // Variable result will hold the final object to return
     const result: MakeContract = {
       abi,
       address,
       get contractObject() {
-        return getContract(address, abiJson, provider);
+        return getContract(address, abiJson, api);
       }
     };
 
@@ -69,7 +71,7 @@ export const makeContract = memoizee(
         // We only get the contract when the function is called for the 1st
         // time. Note: getContract is memoized, won't create contract on each
         // call.
-        const contract = getContract(address, abiJson, provider);
+        const contract = getContract(address, abiJson, api);
         const method = contract.instance[name]; // Hold the method from the Abi
 
         // The last arguments in args can be an options object
@@ -101,6 +103,26 @@ export const makeContract = memoizee(
     });
 
     return result;
-  },
-  { length: 1 } // Only memoize by address
+  }
 );
+
+/**
+ * Create a contract.
+ *
+ * @param address - The contract address.
+ * @param abiJson - The contract abi.
+ * @param options - The options to pass in when creating the contract.
+ * @return - An object whose keys are all the functions of the
+ * contract, and each function return an Observable which will fire when the
+ * function resolves.
+ */
+export const makeContract = (
+  address: Address,
+  abiJson: any[],
+  options: { provider?: any } = {}
+) => {
+  const { provider } = options;
+  const api = provider ? createApiFromProvider(provider) : getApi();
+
+  return makeContractWithApi(address, abiJson, api);
+};

packages/light.js/src/rpc/rpc.ts

@@ -4,11 +4,10 @@
 // SPDX-License-Identifier: MIT
 
 import * as eth from './eth';
-import { memoizeAll } from '../utils/memoizeAll';
 import * as net from './net';
 import { post$ } from './other';
 import * as parity from './parity';
 
 const rpc = { ...eth, ...net, ...parity, post$ };
 
-export default memoizeAll(rpc);
+export default rpc;