Remove ExtenedMap, ToggleSet, and observable versions (#3310)

- Switch to helper functions instead. They don't impact the prototype
  tree and it means that we don't need to introduce versions that are
  both observable and "normal"

- Tidy <ItemListLayout> and hiddenTableColumns to not need
  pre-initialization

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

src/common/catalog/catalog-category-registry.ts

@@ -4,15 +4,16 @@
  */
 
 import { action, computed, observable, makeObservable } from "mobx";
-import { Disposer, ExtendedMap, iter } from "../utils";
+import type { Disposer } from "../utils";
-import { CatalogCategory, CatalogEntityData, CatalogEntityKindData } from "./catalog-entity";
+import { strictSet, iter, getOrInsertMap } from "../utils";
 import { once } from "lodash";
+import { CatalogCategory, CatalogEntityData, CatalogEntityKindData } from "./catalog-entity";
 
 export type CategoryFilter = (category: CatalogCategory) => any;
 
 export class CatalogCategoryRegistry {
   protected categories = observable.set<CatalogCategory>();
-  protected groupKinds = new ExtendedMap<string, ExtendedMap<string, CatalogCategory>>();
+  protected groupKinds = new Map<string, Map<string, CatalogCategory>>();
   protected filters = observable.set<CategoryFilter>([], {
     deep: false,
   });
@@ -22,14 +23,14 @@ export class CatalogCategoryRegistry {
   }
 
   @action add(category: CatalogCategory): Disposer {
+    const byGroup = getOrInsertMap(this.groupKinds, category.spec.group);
+
     this.categories.add(category);
-    this.groupKinds
+    strictSet(byGroup, category.spec.names.kind, category);
-      .getOrInsert(category.spec.group, ExtendedMap.new)
-      .strictSet(category.spec.names.kind, category);
 
     return () => {
       this.categories.delete(category);
-      this.groupKinds.get(category.spec.group).delete(category.spec.names.kind);
+      byGroup.delete(category.spec.names.kind);
     };
   }
 

src/common/utils/collection-functions.ts

@@ -21,8 +21,8 @@ export function getOrInsert<K, V>(map: Map<K, V>, key: K, value: V): V {
 }
 
 /**
- * Like `getOrInsert` but specifically for when `V` is `Map<any, any>` so that
+ * Like `getOrInsert` but specifically for when `V` is `Map<MK, MV>` so that
- * the typings are inferred.
+ * the typings are inferred correctly.
  */
 export function getOrInsertMap<K, MK, MV>(map: Map<K, Map<MK, MV>>, key: K): Map<MK, MV> {
   return getOrInsert(map, key, new Map<MK, MV>());
@@ -37,11 +37,39 @@ export function getOrInsertSet<K, SK>(map: Map<K, Set<SK>>, key: K): Set<SK> {
 }
 
 /**
- * Like `getOrInsert` but with delayed creation of the item
+ * Like `getOrInsert` but with delayed creation of the item. Which is useful
+ * if it is very expensive to create the initial value.
  */
-export function getOrInsertWith<K, V>(map: Map<K, V>, key: K, value: () => V): V {
+export function getOrInsertWith<K, V>(map: Map<K, V>, key: K, builder: () => V): V {
   if (!map.has(key)) {
-    map.set(key, value());
+    map.set(key, builder());
+  }
+
+  return map.get(key);
+}
+
+/**
+ * Set the value associated with `key` iff there was not a previous value
+ * @param map The map to interact with
+ * @throws if `key` already in map
+ * @returns `this` so that `strictSet` can be chained
+ */
+export function strictSet<K, V>(map: Map<K, V>, key: K, val: V): typeof map {
+  if (map.has(key)) {
+    throw new TypeError("Duplicate key in map");
+  }
+
+  return map.set(key, val);
+}
+
+/**
+ * Get the value associated with `key`
+ * @param map The map to interact with
+ * @throws if `key` did not a value associated with it
+ */
+export function strictGet<K, V>(map: Map<K, V>, key: K): V {
+  if (!map.has(key)) {
+    throw new TypeError("key not in map");
   }
 
   return map.get(key);

src/common/utils/extended-map.ts

@@ -1,70 +0,0 @@
-/**
- * Copyright (c) OpenLens Authors. All rights reserved.
- * Licensed under MIT License. See LICENSE in root directory for more information.
- */
-
-import { action, ObservableMap, runInAction } from "mobx";
-
-export function multiSet<T, V>(map: Map<T, V>, newEntries: [T, V][]): void {
-  runInAction(() => {
-    for (const [key, val] of newEntries) {
-      map.set(key, val);
-    }
-  });
-}
-
-export class ExtendedMap<K, V> extends Map<K, V> {
-  static new<K, V>(entries?: readonly (readonly [K, V])[] | null): ExtendedMap<K, V> {
-    return new ExtendedMap<K, V>(entries);
-  }
-
-  /**
-   * Get the value behind `key`. If it was not present, first insert the value returned by `getVal`
-   * @param key The key to insert into the map with
-   * @param getVal A function that returns a new instance of `V`.
-   * @returns The value in the map
-   */
-  getOrInsert(key: K, getVal: () => V): V {
-    if (this.has(key)) {
-      return this.get(key);
-    }
-
-    return this.set(key, getVal()).get(key);
-  }
-
-  /**
-   * Set the value associated with `key` iff there was not a previous value
-   * @throws if `key` already in map
-   * @returns `this` so that `strictSet` can be chained
-   */
-  strictSet(key: K, val: V): this {
-    if (this.has(key)) {
-      throw new TypeError("Duplicate key in map");
-    }
-
-    return this.set(key, val);
-  }
-
-  /**
-   * Get the value associated with `key`
-   * @throws if `key` did not a value associated with it
-   */
-  strictGet(key: K): V {
-    if (!this.has(key)) {
-      throw new TypeError("key not in map");
-    }
-
-    return this.get(key);
-  }
-}
-
-export class ExtendedObservableMap<K, V> extends ObservableMap<K, V> {
-  @action
-  getOrInsert(key: K, getVal: () => V): V {
-    if (this.has(key)) {
-      return this.get(key);
-    }
-
-    return this.set(key, getVal()).get(key);
-  }
-}

src/common/utils/index.ts

@@ -25,7 +25,6 @@ export * from "./delay";
 export * from "./disposer";
 export * from "./downloadFile";
 export * from "./escapeRegExp";
-export * from "./extended-map";
 export * from "./formatDuration";
 export * from "./getRandId";
 export * from "./hash-set";

src/main/catalog-sources/kubeconfig-sync-manager/kubeconfig-sync-manager.ts

@@ -10,7 +10,7 @@ import { FSWatcher, watch } from "chokidar";
 import fs from "fs";
 import path from "path";
 import type stream from "stream";
-import { bytesToUnits, Disposer, ExtendedObservableMap, iter, noop } from "../../../common/utils";
+import { bytesToUnits, Disposer, getOrInsertWith, iter, noop } from "../../../common/utils";
 import logger from "../../logger";
 import type { KubeConfig } from "@kubernetes/client-node";
 import { loadConfigFromString, splitConfig } from "../../../common/kube-helpers";
@@ -294,7 +294,7 @@ const diffChangedConfigFor = (dependencies: Dependencies) => ({ filePath, source
 };
 
 const watchFileChanges = (filePath: string, dependencies: Dependencies): [IComputedValue<CatalogEntity[]>, Disposer] => {
-  const rootSource = new ExtendedObservableMap<string, ObservableMap<string, RootSourceValue>>();
+  const rootSource = observable.map<string, ObservableMap<string, RootSourceValue>>();
   const derivedSource = computed(() => Array.from(iter.flatMap(rootSource.values(), from => iter.map(from.values(), child => child[1]))));
 
   let watcher: FSWatcher;
@@ -335,7 +335,7 @@ const watchFileChanges = (filePath: string, dependencies: Dependencies): [ICompu
           cleanup();
           cleanupFns.set(childFilePath, diffChangedConfig({
             filePath: childFilePath,
-            source: rootSource.getOrInsert(childFilePath, observable.map),
+            source: getOrInsertWith(rootSource, childFilePath, observable.map),
             stats,
             maxAllowedFileReadSize,
           }));
@@ -353,7 +353,7 @@ const watchFileChanges = (filePath: string, dependencies: Dependencies): [ICompu
 
           cleanupFns.set(childFilePath, diffChangedConfig({
             filePath: childFilePath,
-            source: rootSource.getOrInsert(childFilePath, observable.map),
+            source: getOrInsertWith(rootSource, childFilePath, observable.map),
             stats,
             maxAllowedFileReadSize,
           }));

src/main/proxy-functions/shell-api-request/shell-request-authenticator/shell-request-authenticator.ts

@@ -2,7 +2,7 @@
  * Copyright (c) OpenLens Authors. All rights reserved.
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
-import { ExtendedMap } from "../../../../common/utils";
+import { getOrInsertMap } from "../../../../common/utils";
 import type { ClusterId } from "../../../../common/cluster-types";
 import { ipcMainHandle } from "../../../../common/ipc";
 import crypto from "crypto";
@@ -11,15 +11,14 @@ import { promisify } from "util";
 const randomBytes = promisify(crypto.randomBytes);
 
 export class ShellRequestAuthenticator {
-  private tokens = new ExtendedMap<ClusterId, Map<string, Uint8Array>>();
+  private tokens = new Map<ClusterId, Map<string, Uint8Array>>();
 
   init() {
     ipcMainHandle("cluster:shell-api", async (event, clusterId, tabId) => {
       const authToken = Uint8Array.from(await randomBytes(128));
+      const forCluster = getOrInsertMap(this.tokens, clusterId);
 
-      this.tokens
+      forCluster.set(tabId, authToken);
-        .getOrInsert(clusterId, () => new Map())
-        .set(tabId, authToken);
 
       return authToken;
     });

src/migrations/helpers.ts

@@ -5,7 +5,7 @@
 
 import type Conf from "conf";
 import type { Migrations } from "conf/dist/source/types";
-import { ExtendedMap, iter } from "../common/utils";
+import { getOrInsert, iter } from "../common/utils";
 import { isTestEnv } from "../common/vars";
 
 export function migrationLog(...args: any[]) {
@@ -20,10 +20,10 @@ export interface MigrationDeclaration {
 }
 
 export function joinMigrations(...declarations: MigrationDeclaration[]): Migrations<any> {
-  const migrations = new ExtendedMap<string, ((store: Conf<any>) => void)[]>();
+  const migrations = new Map<string, ((store: Conf<any>) => void)[]>();
 
   for (const decl of declarations) {
-    migrations.getOrInsert(decl.version, () => []).push(decl.run);
+    getOrInsert(migrations, decl.version, []).push(decl.run);
   }
 
   return Object.fromEntries(

src/renderer/components/+config-secrets/secret-details.tsx

@@ -27,7 +27,7 @@ interface Props extends KubeObjectDetailsProps<Secret> {
 export class SecretDetails extends React.Component<Props> {
   @observable isSaving = false;
   @observable data: { [name: string]: string } = {};
-  revealSecret = new Set<string>();
+  @observable revealSecret = observable.set<string>();
 
   constructor(props: Props) {
     super(props);

src/renderer/components/+preferences/kubeconfig-syncs.tsx

@@ -11,7 +11,7 @@ import { Notice } from "../+extensions/notice";
 import { KubeconfigSyncEntry, KubeconfigSyncValue, UserStore } from "../../../common/user-store";
 import { isWindows } from "../../../common/vars";
 import logger from "../../../main/logger";
-import { iter, multiSet } from "../../utils";
+import { iter } from "../../utils";
 import { SubTitle } from "../layout/sub-title";
 import { PathPicker } from "../path-picker/path-picker";
 import { Spinner } from "../spinner";
@@ -93,7 +93,9 @@ export class KubeconfigSyncs extends React.Component {
     return Array.from(this.syncs.entries(), ([filePath, value]) => ({ filePath, ...value }));
   }
 
-  onPick = async (filePaths: string[]) => multiSet(this.syncs, await getAllEntries(filePaths));
+  onPick = async (filePaths: string[]) => {
+    this.syncs.merge(await getAllEntries(filePaths));
+  };
 
   getIconName(entry: Entry) {
     switch (entry.info.type) {

src/renderer/initializers/catalog-category-registry.tsx

@@ -6,21 +6,15 @@
 import React from "react";
 import { kubernetesClusterCategory } from "../../common/catalog-entities";
 import { addClusterURL, kubernetesURL } from "../../common/routes";
-import { multiSet } from "../utils";
 import { UserStore } from "../../common/user-store";
 import { getAllEntries } from "../components/+preferences/kubeconfig-syncs";
-import { runInAction } from "mobx";
 import { isLinux, isWindows } from "../../common/vars";
 import { PathPicker } from "../components/path-picker";
 import { Notifications } from "../components/notifications";
 import { Link } from "react-router-dom";
 
 async function addSyncEntries(filePaths: string[]) {
-  const entries = await getAllEntries(filePaths);
+  UserStore.getInstance().syncKubeconfigEntries.merge(await getAllEntries(filePaths));
-
-  runInAction(() => {
-    multiSet(UserStore.getInstance().syncKubeconfigEntries, entries);
-  });
 
   Notifications.ok(
     <div>