feat(app-state): Batch API better reflects its semantics

BREAKING CHANGE: `.batch()` in now only available on `RootStore`. There is a new `Store.getRootStore()` to get a reference to it if needed.

TODO.md

@@ -2,3 +2,6 @@
 - landing page to link to all API docs
 - coveralls
   - help may be here, to combine multiple coverage runs into one report: https://github.com/angular/angular-cli/issues/11268
+
+# App State
+- investigate unsubscribe performance in the `wide-performance.ts`

projects/app-state/README.md

@@ -193,5 +193,6 @@ If you are upgrading from the loose version of [`ng-app-state`](https://github.c
 - Replace references to `AppStore` with `RootStore`.
 - The constructor for `RootStore` takes only 1 argument: its starting state. When transitioning from `AppStore` to `RootStore`, remove the other 2 arguments.
 - Remove calls to `withCaching()`. The redesigned library comes with intelligent caching built in automatically.
-- Batches are no longer tied to a specific slice of the store, but always operate on the entire `RootStore`. Therefore, the callback to `batch()` no longer receives a `Store` object and `.inBatch()` has been removed. Instead, operate on the store using any existing store object and it will automatically be included in the batch.
+- Batches are no longer tied to a specific slice of the store, but always operate on the entire `RootStore`. Therefore, `.batch()` was moved to `RootStore` and its callback no longer receives a `Store` object. Instead, operate on the store using any existing store object and it will automatically be included in the batch. `Store.getRootStore()` was added to get a reference to start the batch from if needed.
+- `.inBatch()` has been removed.
 - The `batch` parameter was removed from `UndoManager.applyUndoState()`. It still operates in a batch, but now you can use `this.store` in its place with no need for the additional argument.

projects/app-state/src/lib/child-store.ts

@@ -1,10 +1,14 @@
 import { clone, isEmpty, omit } from '@s-libs/micro-dash';
-import { Client, Store } from './index';
+import { RootStore, Store } from './index';
 
 /** @hidden */
 export class ChildStore<T> extends Store<T> {
-  constructor(client: Client, private parent: Store<any>, private key: any) {
-    super(client);
+  constructor(
+    getRootStore: () => RootStore<object>,
+    private parent: Store<any>,
+    private key: any,
+  ) {
+    super(getRootStore);
   }
 
   set(value: T): void {
@@ -36,8 +40,8 @@ export class ChildStore<T> extends Store<T> {
   refersToSameStateAs(other: Store<T>): boolean {
     return (
       other instanceof ChildStore &&
-      this.key === (other as ChildStore<T>).key &&
-      this.parent.refersToSameStateAs((other as ChildStore<T>).parent)
+      this.key === other.key &&
+      this.parent.refersToSameStateAs(other.parent)
     );
   }
 

projects/app-state/src/lib/index.ts

@@ -1,4 +1,4 @@
 // this exists to resolve the order of some circular imports
-export { Client, Store } from './store';
+export { Store } from './store';
 export { RootStore } from './root-store';
 export { ChildStore } from './child-store';

projects/app-state/src/lib/root-store.spec.ts

@@ -1,3 +1,4 @@
+import { noop } from '@s-libs/micro-dash';
 import { InnerState, TestState } from '../test-helpers/test-state';
 import { RootStore } from './root-store';
 
@@ -61,6 +62,85 @@ describe('RootStore', () => {
     });
   });
 
+  describe('.batch()', () => {
+    it('causes a single update after multiple actions', () => {
+      const next = jasmine.createSpy();
+
+      store.$.subscribe(next);
+      expect(next).toHaveBeenCalledTimes(1);
+
+      store.batch(() => {
+        store('counter').set(3);
+        store('nested')('state').set(6);
+        expect(next).toHaveBeenCalledTimes(1);
+      });
+
+      expect(next).toHaveBeenCalledTimes(2);
+      expect(store.state()).toEqual({ counter: 3, nested: { state: 6 } });
+    });
+
+    it('works when nested', () => {
+      store.batch(() => {
+        store('counter').set(1);
+        store.batch(() => {
+          expect(store.state().counter).toBe(1);
+          store('counter').set(2);
+          expect(store.state().counter).toBe(2);
+        });
+        expect(store.state().counter).toBe(2);
+      });
+      expect(store.state().counter).toBe(2);
+    });
+
+    it("doesn't have that infinite loop with 2 stores (production bug)", () => {
+      // https://github.com/simontonsoftware/ng-app-state/issues/28
+      const store2 = new RootStore<{}>({});
+      store.$.subscribe(() => {
+        store2.batch(noop);
+      });
+      store2.$.subscribe();
+      store('counter').set(1);
+
+      // the infinite loop was here
+
+      expect(store.state().counter).toBe(1);
+    });
+
+    it("doesn't have that infinite loop with batches (production bug)", () => {
+      // https://github.com/simontonsoftware/ng-app-state/issues/29
+      class State {
+        version = 1;
+        looping = false;
+      }
+      const loopStore = new RootStore<State>(new State());
+      loopStore('version').$.subscribe(() => {
+        loopStore.batch(noop);
+      });
+      loopStore('looping').$.subscribe(() => {
+        loopStore('version').set(2);
+        loopStore('version').set(3);
+      });
+
+      loopStore('looping').set(true);
+      // the infinite loop was here
+
+      expect(loopStore.state().version).toBe(3);
+    });
+
+    it('starts nested batches with the correct state (production bug)', () => {
+      store.batch(() => {
+        store('counter').set(1);
+        store.batch(() => {
+          expect(store.state().counter).toBe(1);
+          store('nested')('state').set(2);
+        });
+      });
+      expect(store.state()).toEqual(
+        jasmine.objectContaining({ counter: 1, nested: { state: 2 } }),
+      );
+    });
+  });
+
   describe('.refersToSameStateAs()', () => {
     it('works', () => {
       expect(store.refersToSameStateAs(store)).toBe(true);

projects/app-state/src/lib/root-store.ts

@@ -4,18 +4,7 @@ export class RootStore<T extends object> extends Store<T> {
   private batchCount = 0;
 
   constructor(state: T) {
-    super({
-      runInBatch: (func: () => void) => {
-        ++this.batchCount;
-        try {
-          func();
-        } finally {
-          if (--this.batchCount === 0) {
-            this.maybeEmit();
-          }
-        }
-      },
-    });
+    super(() => this);
     this.set(state);
   }
 
@@ -32,6 +21,30 @@ export class RootStore<T extends object> extends Store<T> {
     return this.lastKnownState!;
   }
 
+  /**
+   * Turns off this store's observables while `func` is executing, emitting only once afterward, if the store changed. This allows you to batch multiple mutations into a single update at the end.
+   *
+   * ```ts
+   * store.batch(() => {
+   *   store.assign({ key1: value1 });
+   *   store('key2').delete();
+   *   store('key3').set({ key4: value4 });
+   *
+   *   store('key1').state(); // returns `value1`
+   * });
+   * ```
+   */
+  batch(func: VoidFunction): void {
+    ++this.batchCount;
+    try {
+      func();
+    } finally {
+      if (--this.batchCount === 0) {
+        this.maybeEmit();
+      }
+    }
+  }
+
   refersToSameStateAs(other: Store<T>): boolean {
     return this === other;
   }

projects/app-state/src/lib/store.spec.ts

@@ -1,4 +1,4 @@
-import { cloneDeep, identity, noop, pick } from '@s-libs/micro-dash';
+import { cloneDeep, identity, pick } from '@s-libs/micro-dash';
 import { expectSingleCallAndReset } from '@s-libs/ng-dev';
 import { skip, take } from 'rxjs/operators';
 import { InnerState, TestState } from '../test-helpers/test-state';
@@ -193,85 +193,6 @@ describe('Store', () => {
     });
   });
 
-  describe('.batch()', () => {
-    it('causes a single update after multiple actions', () => {
-      const next = jasmine.createSpy();
-
-      store.$.subscribe(next);
-      expect(next).toHaveBeenCalledTimes(1);
-
-      store.batch(() => {
-        store('counter').set(3);
-        store('nested')('state').set(6);
-        expect(next).toHaveBeenCalledTimes(1);
-      });
-
-      expect(next).toHaveBeenCalledTimes(2);
-      expect(store.state()).toEqual({ counter: 3, nested: { state: 6 } });
-    });
-
-    it('works when nested', () => {
-      store.batch(() => {
-        store('counter').set(1);
-        store.batch(() => {
-          expect(store.state().counter).toBe(1);
-          store('counter').set(2);
-          expect(store.state().counter).toBe(2);
-        });
-        expect(store.state().counter).toBe(2);
-      });
-      expect(store.state().counter).toBe(2);
-    });
-
-    it("doesn't have that infinite loop with 2 stores (production bug)", () => {
-      // https://github.com/simontonsoftware/ng-app-state/issues/28
-      const store2 = new RootStore<{}>({});
-      store.$.subscribe(() => {
-        store2.batch(noop);
-      });
-      store2.$.subscribe();
-      store('counter').set(1);
-
-      // the infinite loop was here
-
-      expect(store.state().counter).toBe(1);
-    });
-
-    it("doesn't have that infinite loop with batches (production bug)", () => {
-      // https://github.com/simontonsoftware/ng-app-state/issues/29
-      class State {
-        version = 1;
-        looping = false;
-      }
-      const loopStore = new RootStore<State>(new State());
-      loopStore('version').$.subscribe(() => {
-        loopStore.batch(noop);
-      });
-      loopStore('looping').$.subscribe(() => {
-        loopStore('version').set(2);
-        loopStore('version').set(3);
-      });
-
-      loopStore('looping').set(true);
-      // the infinite loop was here
-
-      expect(loopStore.state().version).toBe(3);
-    });
-
-    it('starts nested batches with the correct state (production bug)', () => {
-      store.batch(() => {
-        store('counter').set(1);
-        store.batch(() => {
-          expect(store.state().counter).toBe(1);
-          store('nested')('state').set(2);
-        });
-      });
-      expect(store.state()).toEqual(
-        jasmine.objectContaining({ counter: 1, nested: { state: 2 } }),
-      );
-    });
-  });
-
   describe('.assign()', () => {
     it('assigns the exact objects given', () => {
       const before = store.state().nested;
@@ -401,6 +322,13 @@ describe('Store', () => {
     });
   });
 
+  describe('.getRootStore()', () => {
+    it('returns a reference to the root store', () => {
+      expect(store.getRootStore()).toBe(store);
+      expect(store('nested')('state').getRootStore()).toBe(store);
+    });
+  });
+
   describe('help for subclasses', () => {
     it('does nothing when trying to update to the same value', () => {
       const startingState = store.state();

projects/app-state/src/lib/store.ts

@@ -1,12 +1,7 @@
 import { CallableObject } from '@s-libs/js-core';
 import { clone, every, forOwn } from '@s-libs/micro-dash';
 import { Observable, Subscriber } from 'rxjs';
-import { ChildStore } from './index';
-
-/** @hidden */
-export interface Client {
-  runInBatch(func: () => void): void;
-}
+import { ChildStore, RootStore } from './index';
 
 /** @hidden */
 type GetSlice<T> = <K extends keyof T>(attr: K) => Store<T[K]>;
@@ -35,16 +30,16 @@ export abstract class Store<T> extends CallableObject<GetSlice<T>> {
   });
 
   protected subscribers = new Map<Subscriber<T>, T | undefined>();
-  protected activeChildren: Record<string, Set<Store<any>>> = {};
+  protected activeChildren: Record<string, Set<Store<unknown>>> = {};
   protected lastKnownState?: T;
 
   private lastKnownStateChanged = false;
 
-  constructor(private client: Client) {
+  constructor(public getRootStore: () => RootStore<object>) {
     super(
       (childKey: any) =>
         this.activeChildren[childKey]?.values().next()?.value ||
-        new ChildStore(client, this, childKey),
+        new ChildStore(getRootStore, this, childKey),
     );
   }
 
@@ -72,24 +67,6 @@ export abstract class Store<T> extends CallableObject<GetSlice<T>> {
    */
   abstract refersToSameStateAs(other: Store<T>): boolean;
 
-  /**
-   * Allows batching multiple mutations on this store object so that observers only receive one event. The batch maintains its own fork of the full global state until it completes, then commits it to the store. Calls to `.state()` on the batch will fetch from the forked state.
-   *
-   * ```ts
-   * store.batch((batch) => {
-   *   batch.assign({ key1: value1 });
-   *   batch('key2').delete();
-   *   batch('key3').set({ key4: value4 });
-   *
-   *   batch('key1').state(); // returns `value1`
-   *   store('key1').state(); // don't do this. may not return `value1`
-   * });
-   * ```
-   */
-  batch(func: () => void): void {
-    this.client.runInBatch(func);
-  }
-
   /**
    * Assigns the given values to state of this store object. The resulting state will be like `Object.assign(store.state(), value)`.
    */

projects/app-state/src/lib/utils/push-to-store-array.spec.ts

@@ -36,7 +36,7 @@ describe('pushToStoreArray', () => {
     let so1: Store<number>;
     let so2: Store<number>;
 
-    store.batch(() => {
+    store.getRootStore().batch(() => {
       so1 = pushToStoreArray(store, 1);
       so2 = pushToStoreArray(store, 2);
 

projects/app-state/src/lib/utils/undo-manager.ts

@@ -189,7 +189,7 @@ export abstract class UndoManager<StateType, UndoStateType> {
     const stateToOverwrite = this.currentUndoState;
     this.currentStateIndex += change;
     const stateToApply = this.currentUndoState;
-    this.store.batch(() => {
+    this.store.getRootStore().batch(() => {
       this.applyUndoState(stateToApply, undoOrRedo, stateToOverwrite);
     });
     this.emitUndoChanges();

projects/app-state/src/typing-tests/store-object.dts-spec.ts → projects/app-state/src/typing-tests/store.dts-spec.ts

@@ -19,3 +19,8 @@ store('obj')('c');
 store('ary');
 // $ExpectType Store<boolean>
 store('ary')(1);
+
+// $ExpectType RootStore<object>
+store.getRootStore();
+// $ExpectType RootStore<object>
+store('obj')('c').getRootStore();