Fix onEffects not called when subscriptions are removed

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased](https://github.com/typescript-tea/core/compare/v0.5.0...master)
 
+### Fixed
+
+- Always call all effect managers so they get updated subscriptions even if there are no subscriptions anymore. See PR [#8](https://github.com/typescript-tea/core/pull/8).
+
 ## [0.5.0](https://github.com/typescript-tea/core/compare/v0.4.0...0.5.0) - 2020-10-22
 
 ### Added

package.json

@@ -39,7 +39,7 @@
     "jest": "^26.6.0",
     "lint-staged": "^10.0.2",
     "microbundle": "^0.12.0-next.8",
-    "prettier": "^2.1.2",
+    "prettier": "^2.3.0",
     "rimraf": "^3.0.0",
     "ts-jest": "^26.4.1",
     "typedoc": "^0.19.2",
@@ -50,6 +50,7 @@
     "clean": "tsc -b --clean && rimraf lib && rimraf dist",
     "test": "jest",
     "test-coverage": "jest --coverage",
+    "test:work": "jest src/__tests__/program.test.ts",
     "lint": "eslint './src/**/*.ts{,x}' --ext .js,.ts,.tsx -f visualstudio",
     "dist": "yarn build && rimraf dist && microbundle src/index.ts",
     "verify": "yarn lint && yarn test-coverage && yarn dist",

src/__tests__/helpers/mock-effect-manager.ts

@@ -0,0 +1,73 @@
+/* eslint-disable functional/prefer-readonly-type */
+import { Cmd } from "../../cmd";
+import { Dispatch } from "../../dispatch";
+import { Sub } from "../../sub";
+
+export type MockHome = "mock";
+export type MockSelfState = number;
+export type MockProgramAction = number;
+export type MockSelfAction = number;
+export type MockCmd<_ignoredA> = never;
+export type MockSub<_ignoredA> = never;
+
+export type MockEffectManager<
+  Home = string,
+  ProgramAction = unknown,
+  SelfAction = unknown,
+  SelfState = unknown,
+  MyCmd extends Cmd<ProgramAction, Home> = Cmd<ProgramAction, Home>,
+  MySub extends Sub<ProgramAction, Home> = Sub<ProgramAction, Home>
+> = {
+  readonly home: Home;
+  // readonly mapCmd: jest.Mock<ReturnType<EffectManager["mapCmd"]>, Parameters<EffectManager["mapCmd"]>>;
+  // eslint-disable-next-line functional/prefer-readonly-type,@typescript-eslint/no-explicit-any
+  readonly mapCmd: jest.Mock<Cmd<any>, [actionMapper: (a1: any) => any, cmd: Cmd<any>]>;
+  // readonly mapSub: jest.Mock<ReturnType<EffectManager["mapSub"]>, Parameters<EffectManager["mapSub"]>>;
+  // eslint-disable-next-line functional/prefer-readonly-type,@typescript-eslint/no-explicit-any
+  readonly mapSub: jest.Mock<Sub<any>, [actionMapper: (a1: any) => any, cmd: Sub<any>]>;
+  readonly setup: jest.Mock<() => void, [dispatchProgram: Dispatch<ProgramAction>, dispatchSelf: Dispatch<SelfAction>]>;
+  readonly onEffects: jest.Mock<
+    SelfState,
+    [
+      dispatchProgram: Dispatch<ProgramAction>,
+      dispatchSelf: Dispatch<SelfAction>,
+      cmds: ReadonlyArray<MyCmd>,
+      subs: ReadonlyArray<MySub>,
+      state: SelfState
+    ]
+  >;
+  readonly onSelfAction: jest.Mock<
+    SelfState,
+    [dispatchProgram: Dispatch<ProgramAction>, dispatchSelf: Dispatch<SelfAction>, action: SelfAction, state: SelfState]
+  >;
+};
+
+export function createMockEffectManager<THome extends string>(home: THome): MockEffectManager {
+  return {
+    home,
+    mapCmd: jest.fn(<A1, A2>(_actionMapper: (a: A1) => A2, cmd: MockCmd<A1>): MockCmd<A2> => cmd),
+    mapSub: jest.fn(<A1, A2>(_actionMapper: (a: A1) => A2, sub: MockSub<A1>): MockSub<A2> => sub),
+    onEffects: jest.fn(
+      (
+        _dispatchProgram: Dispatch<MockProgramAction>,
+        _dispatchSelf: Dispatch<MockSelfAction>,
+        _cmds: ReadonlyArray<MockCmd<MockProgramAction>>,
+        _subs: ReadonlyArray<MockSub<MockProgramAction>>,
+        _state: MockSelfState
+      ): MockSelfState => 0
+    ),
+    onSelfAction: jest.fn(
+      (
+        _dispatchProgram: Dispatch<MockProgramAction>,
+        _dispatchSelf: Dispatch<MockSelfAction>,
+        _action: MockSelfAction,
+        _state: MockSelfState
+      ): MockSelfState => 0
+    ),
+    setup: jest.fn(
+      (_dispatchProgram: Dispatch<MockProgramAction>, _dispatchSelf: Dispatch<MockSelfAction>): (() => void) =>
+        () =>
+          0
+    ),
+  };
+}

src/__tests__/helpers/mock-program.ts

@@ -0,0 +1,22 @@
+/* eslint-disable functional/prefer-readonly-type */
+import { Cmd } from "../../cmd";
+import { Dispatch } from "../../dispatch";
+import { Sub } from "../../sub";
+
+export type MockProgram<Init, State, Action, View> = {
+  readonly init: jest.Mock<readonly [State, Cmd<Action>?], [init: Init]>;
+  readonly update: jest.Mock<readonly [State, Cmd<Action>?], [action: Action, state: State]>;
+  readonly view: jest.Mock<View, [props: { readonly state: State; readonly dispatch: Dispatch<Action> }]>;
+  readonly subscriptions: jest.Mock<Sub<Action> | undefined, [state: State]>;
+};
+
+export function createMockProgram<Init, State, Action, View>(): MockProgram<Init, State, Action, View> {
+  const init = jest.fn((_init: Init) => [0 as unknown as State] as const);
+  const update = jest.fn((_action: Action, state: State) => [state] as const);
+  const view = jest.fn(
+    (_props: { readonly state: State; readonly dispatch: Dispatch<Action> }) => 0 as unknown as View
+  );
+  const subscriptions = jest.fn((_state: State) => undefined);
+  const mp: MockProgram<Init, State, Action, View> = { init, update, view, subscriptions };
+  return mp;
+}

src/__tests__/helpers/mock-render.ts

@@ -0,0 +1,5 @@
+export type MockRender<View> = (view: View) => void;
+
+export function createMockRender<View>(): MockRender<View> {
+  return jest.fn(() => "");
+}

src/__tests__/program.test.ts

@@ -1,4 +1,8 @@
+/* eslint-disable functional/prefer-readonly-type */
 import { Program, run } from "../program";
+import { createMockEffectManager } from "./helpers/mock-effect-manager";
+import { createMockProgram } from "./helpers/mock-program";
+import { createMockRender } from "./helpers/mock-render";
 
 beforeAll(() => {
   globalThis.window = {
@@ -31,21 +35,63 @@ test("Run simple program", () => {
 });
 
 test("View can dispatch", (done) => {
-  const render = (): void => {
-    // Do nothing
-  };
-  const program: Program<undefined, number, string, string> = {
-    init: () => [0],
-    update: () => [1],
-    view: ({ dispatch, state }) => {
-      if (state === 0) {
-        dispatch("increment");
-      } else {
-        expect(state).toEqual(1);
-        done();
-      }
-      return "view";
-    },
-  };
-  run(program, undefined, render, []);
+  // Create mocks
+  const mp = createMockProgram();
+  const mr = createMockRender();
+  // Setup mokcs
+  mp.update.mockImplementationOnce(() => [1]);
+  mp.view
+    .mockImplementationOnce(({ dispatch }) => dispatch("increment"))
+    .mockImplementationOnce(({ state }) => {
+      expect(state).toEqual(1);
+      done();
+    });
+  // Run
+  run(mp, undefined, mr, []);
+});
+
+test("onEffects is called when subscriptions is not undefined", (done) => {
+  // Create mocks
+  const emHome = "mock1" as const;
+  const me = createMockEffectManager(emHome);
+  const mp = createMockProgram();
+  const mr = createMockRender();
+  // Setup mocks
+  mp.update.mockImplementationOnce(() => [1]);
+  mp.subscriptions.mockReturnValue({ home: emHome, type: "nisse" });
+  mp.view
+    .mockImplementationOnce(({ dispatch }) => dispatch("increment"))
+    .mockImplementationOnce(({ state }) => {
+      expect(state).toEqual(1);
+      expect(me.onEffects.mock.calls.length).toBe(2);
+      done();
+    });
+  me.onEffects.mockReturnValueOnce(0);
+  // Run
+  run(mp, undefined, mr, [me]);
+});
+
+/**
+ * onEffects must be called with undefined subscriptions becuase
+ * the previous call may have had subscriptions so teh effect
+ * manager must know to clear those subscriptions when undefined
+ * is returned from program.subscription().
+ */
+test("onEffects is called when subscriptions is undefined", (done) => {
+  // Create mocks
+  const emHome = "mock1" as const;
+  const me = createMockEffectManager(emHome);
+  const mp = createMockProgram();
+  const mr = createMockRender();
+  // Setup mocks
+  mp.update.mockImplementationOnce(() => [1]);
+  mp.subscriptions.mockReturnValueOnce(undefined);
+  mp.view.mockImplementationOnce(({ state }) => {
+    expect(state).toEqual(0);
+    expect(me.onEffects.mock.calls.length).toBe(1);
+    done();
+  });
+  me.onEffects.mockReturnValueOnce(0);
+  // Run
+  run(mp, undefined, mr, [me]);
 });

src/program.ts

@@ -54,23 +54,32 @@ export function run<Init, State, Action, View>(
     isProcessing = false;
   }
 
-  const dispatchManager = (home: string) => (action: Action): void => {
-    if (isRunning) {
-      const manager = getEffectManager(home);
-      const enqueueSelfAction = enqueueManagerAction(home);
-      managerStates[home] = manager.onSelfAction(enqueueProgramAction, enqueueSelfAction, action, managerStates[home]);
-    }
-  };
+  const dispatchManager =
+    (home: string) =>
+    (action: Action): void => {
+      if (isRunning) {
+        const manager = getEffectManager(home);
+        const enqueueSelfAction = enqueueManagerAction(home);
+        managerStates[home] = manager.onSelfAction(
+          enqueueProgramAction,
+          enqueueSelfAction,
+          action,
+          managerStates[home]
+        );
+      }
+    };
 
   function dispatchApp(action: Action): void {
     if (isRunning) {
       change(update(action, state));
     }
   }
 
-  const enqueueManagerAction = (home: string) => (action: unknown): void => {
-    enqueueRaw(dispatchManager(home), action);
-  };
+  const enqueueManagerAction =
+    (home: string) =>
+    (action: unknown): void => {
+      enqueueRaw(dispatchManager(home), action);
+    };
 
   const enqueueProgramAction = (action: Action): void => {
     enqueueRaw(dispatchApp, action);
@@ -90,8 +99,10 @@ export function run<Init, State, Action, View>(
     const gatheredEffects: GatheredEffects<Action> = {};
     cmd && gatherEffects(getEffectManager, gatheredEffects, true, cmd); // eslint-disable-line @typescript-eslint/no-unused-expressions,no-unused-expressions
     sub && gatherEffects(getEffectManager, gatheredEffects, false, sub); // eslint-disable-line @typescript-eslint/no-unused-expressions,no-unused-expressions
-    for (const home of Object.keys(gatheredEffects)) {
-      const { cmds, subs } = gatheredEffects[home];
+    // Always call all effect managers so they get updated subscriptions even if there are no subscriptions anymore
+    for (const em of effectManagers) {
+      const home = em.home;
+      const { cmds, subs } = gatheredEffects[home] ?? {};
       const manager = getEffectManager(home);
       managerStates[home] = manager.onEffects(
         enqueueProgramAction,

yarn.lock

@@ -5833,10 +5833,10 @@ prelude-ls@~1.1.2:
   resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.1.2.tgz#21932a549f5e52ffd9a827f570e04be62a97da54"
   integrity sha1-IZMqVJ9eUv/ZqCf1cOBL5iqX2lQ=
 
-prettier@^2.1.2:
-  version "2.1.2"
-  resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.1.2.tgz#3050700dae2e4c8b67c4c3f666cdb8af405e1ce5"
-  integrity sha512-16c7K+x4qVlJg9rEbXl7HEGmQyZlG4R9AgP+oHKRMsMsuk8s+ATStlf1NpDqyBI1HpVyfjLOeMhH2LvuNvV5Vg==
+prettier@^2.3.0:
+  version "2.3.0"
+  resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.3.0.tgz#b6a5bf1284026ae640f17f7ff5658a7567fc0d18"
+  integrity sha512-kXtO4s0Lz/DW/IJ9QdWhAf7/NmPWQXkFr/r/WkR3vyI+0v8amTDxiaQSLzs8NBlytfLWX/7uQUMIW677yLKl4w==
 
 pretty-bytes@^3.0.0:
   version "3.0.1"