Remove reaction and replace with ObservableCrate

- This new abstraction allows us to hook into the transitions between
  values without having to resort to reactions. Allowing us to be
  explicit in when we want code to execute and also be defensive against
  new code paths

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/common/utils/index.ts

@@ -3,13 +3,6 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 
-/**
- * A function that does nothing
- */
-export function noop<T extends any[]>(...args: T): void {
-  return void args;
-}
-
 export * from "./abort-controller";
 export * from "./app-version";
 export * from "./autobind";
@@ -27,6 +20,8 @@ export * from "./formatDuration";
 export * from "./getRandId";
 export * from "./hash-set";
 export * from "./n-fircate";
+export * from "./noop";
+export * from "./observable-crate/impl";
 export * from "./openBrowser";
 export * from "./paths";
 export * from "./promise-exec";

src/common/utils/noop.ts

@@ -0,0 +1,10 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+/**
+ * A function that does nothing
+ */
+export function noop<T extends any[]>(...args: T): void {
+  return void args;
+}

src/common/utils/observable-crate/impl.ts

@@ -0,0 +1,53 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+import { observable, runInAction } from "mobx";
+import { getOrInsertMap } from "../collection-functions";
+import { noop } from "../noop";
+
+export interface ObservableCrate<T> {
+  get(): T;
+  set(value: T): void;
+}
+
+export interface ObservableCrateFactory {
+  <T>(initialValue: T, transitionHandlers?: ObservableCrateTransitionHandlers<T>): ObservableCrate<T>;
+}
+
+export interface ObservableCrateTransitionHandler<T> {
+  from: T;
+  to: T;
+  onTransition: () => void;
+}
+export type ObservableCrateTransitionHandlers<T> = ObservableCrateTransitionHandler<T>[];
+
+function convertToHandlersMap<T>(handlers: ObservableCrateTransitionHandlers<T>): Map<T, Map<T, () => void>> {
+  const res: ReturnType<typeof convertToHandlersMap<T>> = new Map();
+
+  for (const { from, to, onTransition } of handlers) {
+    getOrInsertMap(res, from).set(to, onTransition);
+  }
+
+  return res;
+}
+
+export const observableCrate = ((initialValue, transitionHandlers = []) => {
+  const crate = observable.box(initialValue);
+  const handlers = convertToHandlersMap(transitionHandlers);
+
+  return {
+    get() {
+      return crate.get();
+    },
+    set(value) {
+      const onTransition = handlers.get(crate.get())?.get(value) ?? noop;
+
+      runInAction(() => {
+        crate.set(value);
+        onTransition();
+      });
+    },
+  };
+}) as ObservableCrateFactory;

src/common/utils/observable-crate/observable-crate.test.ts

@@ -0,0 +1,86 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+import type { ObservableCrate } from "./impl";
+import { observableCrate } from "./impl";
+
+describe("observable-crate", () => {
+  it("can be constructed with initial value", () => {
+    expect(() => observableCrate(0).build()).not.toThrow();
+  });
+
+  it("has a definite type if the initial value is provided", () => {
+    expect (() => {
+      const res: ObservableCrate<number> = observableCrate(0).build();
+
+      void res;
+    }).not.toThrow();
+  });
+
+  it("accepts a map of transitionHandlers", () => {
+    expect(() => observableCrate(0).withHandlers(new Map())).not.toThrow();
+  });
+
+  describe("with a crate over an enum, and some transition handlers", () => {
+    enum Test {
+      Start,
+      T1,
+      End,
+    }
+
+    let crate: ObservableCrate<Test>;
+    let correctHandler: jest.MockedFunction<() => void>;
+    let incorrectHandler: jest.MockedFunction<() => void>;
+
+    beforeEach(() => {
+      correctHandler = jest.fn();
+      incorrectHandler = jest.fn();
+      crate = observableCrate(Test.Start).withHandlers(new Map([
+        [Test.Start, new Map([
+          [Test.Start, incorrectHandler],
+          [Test.T1, correctHandler],
+          [Test.End, incorrectHandler],
+        ])],
+        [Test.T1, new Map([
+          [Test.Start, incorrectHandler],
+          [Test.T1, incorrectHandler],
+          [Test.End, incorrectHandler],
+        ])],
+        [Test.End, new Map([
+          [Test.Start, incorrectHandler],
+          [Test.T1, incorrectHandler],
+          [Test.End, incorrectHandler],
+        ])],
+      ]));
+    });
+
+    it("initial value is available", () => {
+      expect(crate.get()).toBe(Test.Start);
+    });
+
+    it("does not call any transition handler", () => {
+      expect(correctHandler).not.toBeCalled();
+      expect(incorrectHandler).not.toBeCalled();
+    });
+
+    describe("when setting a new value", () => {
+      beforeEach(() => {
+        crate.set(Test.T1);
+      });
+
+      it("calls the associated transition handler", () => {
+        expect(correctHandler).toBeCalled();
+      });
+
+      it("does not call any other transition handler", () => {
+        expect(incorrectHandler).not.toBeCalled();
+      });
+
+      it("new value is available", () => {
+        expect(crate.get()).toBe(Test.T1);
+      });
+    });
+  });
+});

src/renderer/components/+namespaces/namespace-select-filter-model/is-selection-key.injectable.ts

@@ -0,0 +1,22 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectable } from "@ogre-tools/injectable";
+import type React from "react";
+import isMacInjectable from "../../../../common/vars/is-mac.injectable";
+
+export type IsMultiSelectionKey = (event: React.KeyboardEvent) => boolean;
+
+const isMultiSelectionKeyInjectable = getInjectable({
+  id: "is-multi-selection-key",
+  instantiate: (di): IsMultiSelectionKey => {
+    const isMac = di.inject(isMacInjectable);
+
+    return isMac
+      ? ({ key }) => key === "Meta"
+      : ({ key }) => key === "Control";
+  },
+});
+
+export default isMultiSelectionKeyInjectable;

src/renderer/components/+namespaces/namespace-select-filter-model/namespace-select-filter-model.injectable.ts

@@ -2,17 +2,17 @@
  * Copyright (c) OpenLens Authors. All rights reserved.
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
-import { NamespaceSelectFilterModel } from "./namespace-select-filter-model";
+import { namespaceSelectFilterModelFor } from "./namespace-select-filter-model";
 import { getInjectable } from "@ogre-tools/injectable";
 import namespaceStoreInjectable from "../store.injectable";
-import isMacInjectable from "../../../../common/vars/is-mac.injectable";
+import isMultiSelectionKeyInjectable from "./is-selection-key.injectable";
 
 const namespaceSelectFilterModelInjectable = getInjectable({
   id: "namespace-select-filter-model",
 
-  instantiate: (di) => new NamespaceSelectFilterModel({
+  instantiate: (di) => namespaceSelectFilterModelFor({
     namespaceStore: di.inject(namespaceStoreInjectable),
-    isMac: di.inject(isMacInjectable),
+    isMultiSelectionKey: di.inject(isMultiSelectionKeyInjectable),
   }),
 });
 

src/renderer/components/+namespaces/namespace-select-filter-model/namespace-select-filter-model.tsx

@@ -3,16 +3,18 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 import React from "react";
-import { observable, action, computed, makeObservable, comparer, reaction } from "mobx";
+import type { IComputedValue } from "mobx";
+import { observable, action, computed, comparer } from "mobx";
 import type { NamespaceStore } from "../store";
-import type { ActionMeta } from "react-select";
+import type { ActionMeta, MultiValue } from "react-select";
 import { Icon } from "../../icon";
 import type { SelectOption } from "../../select";
-import { autoBind } from "../../../utils";
+import { observableCrate } from "../../../utils";
+import type { IsMultiSelectionKey } from "./is-selection-key.injectable";
 
 interface Dependencies {
-  readonly namespaceStore: NamespaceStore;
-  readonly isMac: boolean;
+  namespaceStore: NamespaceStore;
+  isMultiSelectionKey: IsMultiSelectionKey;
 }
 
 export const selectAllNamespaces = Symbol("all-namespaces-selected");
@@ -20,71 +22,9 @@ export const selectAllNamespaces = Symbol("all-namespaces-selected");
 export type SelectAllNamespaces = typeof selectAllNamespaces;
 export type NamespaceSelectFilterOption = SelectOption<string | SelectAllNamespaces>;
 
-export class NamespaceSelectFilterModel {
-  private isSelectionKey = (event: React.KeyboardEvent): boolean  => {
-    if (this.dependencies.isMac) {
-      return event.key === "Meta";
-    }
-
-    return event.key === "Control"; // windows or linux
-  };
-
-  constructor(private readonly dependencies: Dependencies) {
-    makeObservable(this);
-    autoBind(this);
-
-    reaction(
-      () => this.menuIsOpen.get(),
-      (isOpen) => {
-        if (!isOpen) { // falling edge of menu being open
-          this.optionsSortingSelected.replace(this.selectedNames.get());
-          this.didToggle = false;
-        }
-      },
-    );
-  }
-
-  readonly menuIsOpen = observable.box(false);
-
-  private readonly selectedNames = computed(() => new Set(this.dependencies.namespaceStore.contextNamespaces), {
-    equals: comparer.structural,
-  });
-
-  /**
-   * This set is only updated on the falling edge of the menu being open. That way while the menu is
-   * open the order of the items doesn't change
-   */
-  private readonly optionsSortingSelected = observable.set<string>(this.selectedNames.get());
-
-  readonly options = computed((): readonly NamespaceSelectFilterOption[] => {
-    const baseOptions = this.dependencies.namespaceStore.items.map(ns => ns.getName());
-    const selectedNames = this.selectedNames.get();
-
-    baseOptions.sort((
-      (left, right) =>
-        +this.optionsSortingSelected.has(right)
-        - +this.optionsSortingSelected.has(left)
-    ));
-
-    return [
-      {
-        value: selectAllNamespaces,
-        label: "All Namespaces",
-        id: "all-namespaces",
-        isSelected: false,
-      },
-      ...baseOptions.map(namespace => ({
-        value: namespace,
-        label: namespace,
-        id: namespace,
-        isSelected: selectedNames.has(namespace),
-      })),
-    ];
-  });
-
-  formatOptionLabel({ value, isSelected }: NamespaceSelectFilterOption) {
+export function formatOptionLabel({ value, isSelected }: NamespaceSelectFilterOption) {
   if (value === selectAllNamespaces) {
-      return "All Namespaces";
+    return <>All Namespaces</>;
   }
 
   return (
@@ -103,87 +43,135 @@ export class NamespaceSelectFilterModel {
   );
 }
 
-  @action
-  closeMenu() {
-    this.menuIsOpen.set(false);
+export interface NamespaceSelectFilterModel {
+  readonly options: IComputedValue<readonly NamespaceSelectFilterOption[]>;
+  readonly menu: {
+    open: () => void;
+    close: () => void;
+    readonly isOpen: boolean;
+  };
+  onChange: (newValue: MultiValue<NamespaceSelectFilterOption>, actionMeta: ActionMeta<NamespaceSelectFilterOption>) => void;
+  onClick: () => void;
+  onKeyDown: React.KeyboardEventHandler;
+  onKeyUp: React.KeyboardEventHandler;
+  reset: () => void;
 }
 
-  @action
-  openMenu(){
-    this.menuIsOpen.set(true);
+enum SelectMenuState {
+  Close = "close",
+  Open = "open",
 }
 
-  isSelected(namespace: string | string[]) {
-    return this.dependencies.namespaceStore.hasContext(namespace);
-  }
+export function namespaceSelectFilterModelFor(dependencies: Dependencies): NamespaceSelectFilterModel {
+  const { isMultiSelectionKey, namespaceStore } = dependencies;
 
-  selectSingle(namespace: string) {
-    this.dependencies.namespaceStore.selectSingle(namespace);
-  }
+  let didToggle = false;
+  let isMultiSelection = false;
+  const menuState = observableCrate(SelectMenuState.Close, [{
+    from: SelectMenuState.Close,
+    to: SelectMenuState.Open,
+    onTransition: () => {
+      optionsSortingSelected.replace(selectedNames.get());
+      didToggle = false;
+    },
+  }]);
+  const selectedNames = computed(() => new Set(namespaceStore.contextNamespaces), {
+    equals: comparer.structural,
+  });
+  const optionsSortingSelected = observable.set(selectedNames.get());
+  const options = computed((): readonly NamespaceSelectFilterOption[] => {
+    const baseOptions = namespaceStore.items.map(ns => ns.getName());
+    const namespaces = selectedNames.get();
 
-  selectAll() {
-    this.dependencies.namespaceStore.selectAll();
-  }
+    baseOptions.sort((
+      (left, right) =>
+        +optionsSortingSelected.has(right)
+        - +optionsSortingSelected.has(left)
+    ));
 
-  onChange(namespace: unknown, action: ActionMeta<NamespaceSelectFilterOption>) {
+    return [
+      {
+        value: selectAllNamespaces,
+        label: "All Namespaces",
+        id: "all-namespaces",
+        isSelected: false,
+      },
+      ...baseOptions.map(namespace => ({
+        value: namespace,
+        label: namespace,
+        id: namespace,
+        isSelected: namespaces.has(namespace),
+      })),
+    ];
+  });
+  const menuIsOpen = computed(() => menuState.get() === SelectMenuState.Open);
+
+  const model: NamespaceSelectFilterModel = {
+    options,
+    menu: {
+      close: action(() => {
+        menuState.set(SelectMenuState.Close);
+      }),
+      open: action(() => {
+        menuState.set(SelectMenuState.Open);
+      }),
+      get isOpen() {
+        return menuIsOpen.get();
+      },
+    },
+    onChange: (_, action) => {
       switch (action.action) {
         case "clear":
-        this.dependencies.namespaceStore.selectAll();
+          namespaceStore.selectAll();
           break;
         case "deselect-option":
           if (typeof action.option === "string") {
-          this.didToggle = true;
-          this.dependencies.namespaceStore.toggleSingle(action.option);
+            didToggle = true;
+            namespaceStore.toggleSingle(action.option);
           }
           break;
         case "select-option":
           if (action.option?.value === selectAllNamespaces) {
-          this.didToggle = true;
-          this.dependencies.namespaceStore.selectAll();
+            didToggle = true;
+            namespaceStore.selectAll();
           } else if (action.option) {
-          this.didToggle = true;
+            didToggle = true;
 
-          if (this.isMultiSelection) {
-            this.dependencies.namespaceStore.toggleSingle(action.option.value);
+            if (isMultiSelection) {
+              namespaceStore.toggleSingle(action.option.value);
             } else {
-            this.dependencies.namespaceStore.selectSingle(action.option.value);
+              namespaceStore.selectSingle(action.option.value);
             }
           }
           break;
       }
+    },
+    onClick: () => {
+      if (!menuIsOpen.get()) {
+        model.menu.open();
+      } else if (!isMultiSelection) {
+        model.menu.close();
       }
+    },
+    onKeyDown: (event) => {
+      if (isMultiSelectionKey(event)) {
+        isMultiSelection = true;
+      }
+    },
+    onKeyUp: (event) => {
+      if (isMultiSelectionKey(event)) {
+        isMultiSelection = false;
 
-  onClick() {
-    if (!this.menuIsOpen.get()) {
-      this.openMenu();
-    } else if (!this.isMultiSelection) {
-      this.closeMenu();
+        if (didToggle) {
+          model.menu.close();
         }
       }
+    },
+    reset: action(() => {
+      isMultiSelection = false;
+      model.menu.close();
+    }),
+  };
 
-  private isMultiSelection = false;
-
-  onKeyDown(event: React.KeyboardEvent) {
-    if (this.isSelectionKey(event)) {
-      this.isMultiSelection = true;
-    }
-  }
-
-  private didToggle = false;
-
-  onKeyUp(event: React.KeyboardEvent) {
-    if (this.isSelectionKey(event)) {
-      this.isMultiSelection = false;
-
-      if (this.didToggle) {
-        this.closeMenu();
-      }
-    }
-  }
-
-  @action
-  reset() {
-    this.isMultiSelection = false;
-    this.closeMenu();
-  }
+  return model;
 }

src/renderer/components/+namespaces/namespace-select-filter.tsx

@@ -13,6 +13,7 @@ import type { NamespaceStore } from "./store";
 import { Select } from "../select";
 import { withInjectables } from "@ogre-tools/injectable-react";
 import type { NamespaceSelectFilterModel, NamespaceSelectFilterOption, SelectAllNamespaces } from "./namespace-select-filter-model/namespace-select-filter-model";
+import { formatOptionLabel } from "./namespace-select-filter-model/namespace-select-filter-model";
 import namespaceSelectFilterModelInjectable from "./namespace-select-filter-model/namespace-select-filter-model.injectable";
 import namespaceStoreInjectable from "./store.injectable";
 
@@ -36,13 +37,13 @@ const NonInjectedNamespaceSelectFilter = observer(({ model, id }: Dependencies &
       id={id}
       isMulti={true}
       isClearable={false}
-      menuIsOpen={model.menuIsOpen.get()}
+      menuIsOpen={model.menu.isOpen}
       components={{ Placeholder }}
       closeMenuOnSelect={false}
       controlShouldRenderValue={false}
       onChange={model.onChange}
       onBlur={model.reset}
-      formatOptionLabel={model.formatOptionLabel}
+      formatOptionLabel={formatOptionLabel}
       options={model.options.get()}
       className="NamespaceSelect NamespaceSelectFilter"
       menuClass="NamespaceSelectFilterMenu"