Only create TerminalStore after ThemeStore (#3359)

- Use Proxy for backwards compatability

- Fix a bunch of unit tests that needed all the Singleton's created

- Add comments on the order required for the store migrations

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

package.json

@@ -63,8 +63,7 @@
     },
     "moduleNameMapper": {
       "\\.(css|scss)$": "<rootDir>/__mocks__/styleMock.ts",
-      "\\.(svg)$": "<rootDir>/__mocks__/imageMock.ts",
-      "^@lingui/macro$": "<rootDir>/__mocks__/@linguiMacro.ts"
+      "\\.(svg)$": "<rootDir>/__mocks__/imageMock.ts"
     },
     "modulePathIgnorePatterns": [
       "<rootDir>/dist",
@@ -218,7 +217,7 @@
     "mobx": "^6.3.0",
     "mobx-observable-history": "^2.0.1",
     "mobx-react": "^7.1.0",
-    "mock-fs": "^4.12.0",
+    "mock-fs": "4.14",
     "moment": "^2.29.1",
     "moment-timezone": "^0.5.33",
     "node-pty": "^0.9.0",
@@ -279,7 +278,7 @@
     "@types/marked": "^2.0.3",
     "@types/md5-file": "^4.0.2",
     "@types/mini-css-extract-plugin": "^0.9.1",
-    "@types/mock-fs": "^4.10.0",
+    "@types/mock-fs": "^4.13.1",
     "@types/module-alias": "^2.0.0",
     "@types/node": "12.20",
     "@types/npm": "^2.0.31",
@@ -339,7 +338,7 @@
     "html-webpack-plugin": "^4.5.2",
     "identity-obj-proxy": "^3.0.0",
     "include-media": "^1.4.9",
-    "jest": "^26.0.1",
+    "jest": "26.6.3",
     "jest-canvas-mock": "^2.3.0",
     "jest-fetch-mock": "^3.0.3",
     "jest-mock-extended": "^1.0.16",

src/extensions/registries/__tests__/page-registry.test.ts

@@ -22,14 +22,24 @@
 import { ClusterPageRegistry, getExtensionPageUrl, GlobalPageRegistry, PageParams } from "../page-registry";
 import { LensExtension } from "../../lens-extension";
 import React from "react";
+import fse from "fs-extra";
 import { Console } from "console";
 import { stdout, stderr } from "process";
+import { ThemeStore } from "../../../renderer/theme.store";
+import { TerminalStore } from "../../renderer-api/components";
+import { UserStore } from "../../../common/user-store";
+
+jest.mock("electron", () => ({
+  app: {
+    getPath: () => "tmp",
+  },
+}));
 
 console = new Console(stdout, stderr);
 
 let ext: LensExtension = null;
 
-describe("getPageUrl", () => {
+describe("page registry tests", () => {
   beforeEach(async () => {
     ext = new LensExtension({
       manifest: {
@@ -43,6 +53,9 @@ describe("getPageUrl", () => {
       isEnabled: true,
       isCompatible: true
     });
+    UserStore.createInstance();
+    ThemeStore.createInstance();
+    TerminalStore.createInstance();
     ClusterPageRegistry.createInstance();
     GlobalPageRegistry.createInstance().add({
       id: "page-with-params",
@@ -54,70 +67,6 @@ describe("getPageUrl", () => {
         test2: "" // no default value, just declaration
       },
     }, ext);
-  });
-
-  afterEach(() => {
-    GlobalPageRegistry.resetInstance();
-    ClusterPageRegistry.resetInstance();
-  });
-
-  it("returns a page url for extension", () => {
-    expect(getExtensionPageUrl({ extensionId: ext.name })).toBe("/extension/foo-bar");
-  });
-
-  it("allows to pass base url as parameter", () => {
-    expect(getExtensionPageUrl({ extensionId: ext.name, pageId: "/test" })).toBe("/extension/foo-bar/test");
-  });
-
-  it("removes @ and replace `/` to `--`", () => {
-    expect(getExtensionPageUrl({ extensionId: "@foo/bar" })).toBe("/extension/foo--bar");
-  });
-
-  it("adds / prefix", () => {
-    expect(getExtensionPageUrl({ extensionId: ext.name, pageId: "test" })).toBe("/extension/foo-bar/test");
-  });
-
-  it("normalize possible multi-slashes in page.id", () => {
-    expect(getExtensionPageUrl({ extensionId: ext.name, pageId: "//test/" })).toBe("/extension/foo-bar/test");
-  });
-
-  it("gets page url with custom params", () => {
-    const params: PageParams = { test1: "one", test2: "2" };
-    const searchParams = new URLSearchParams(params);
-    const pageUrl = getExtensionPageUrl({
-      extensionId: ext.name,
-      pageId: "page-with-params",
-      params,
-    });
-
-    expect(pageUrl).toBe(`/extension/foo-bar/page-with-params?${searchParams}`);
-  });
-
-  it("gets page url with default custom params", () => {
-    const defaultPageUrl = getExtensionPageUrl({
-      extensionId: ext.name,
-      pageId: "page-with-params",
-    });
-
-    expect(defaultPageUrl).toBe(`/extension/foo-bar/page-with-params?test1=test1-default`);
-  });
-});
-
-describe("globalPageRegistry", () => {
-  beforeEach(async () => {
-    ext = new LensExtension({
-      manifest: {
-        name: "@acme/foo-bar",
-        version: "0.1.1"
-      },
-      id: "/this/is/fake/package.json",
-      absolutePath: "/absolute/fake/",
-      manifestPath: "/this/is/fake/package.json",
-      isBundled: false,
-      isEnabled: true,
-      isCompatible: true
-    });
-    ClusterPageRegistry.createInstance();
     GlobalPageRegistry.createInstance().add([
       {
         id: "test-page",
@@ -142,33 +91,82 @@ describe("globalPageRegistry", () => {
   afterEach(() => {
     GlobalPageRegistry.resetInstance();
     ClusterPageRegistry.resetInstance();
+    TerminalStore.resetInstance();
+    ThemeStore.resetInstance();
+    UserStore.resetInstance();
+    fse.remove("tmp");
   });
 
-  describe("getByPageTarget", () => {
-    it("matching to first registered page without id", () => {
-      const page = GlobalPageRegistry.getInstance().getByPageTarget({ extensionId: ext.name });
-
-      expect(page.id).toEqual(undefined);
-      expect(page.extensionId).toEqual(ext.name);
-      expect(page.url).toEqual(getExtensionPageUrl({ extensionId: ext.name }));
+  describe("getPageUrl", () => {
+    it("returns a page url for extension", () => {
+      expect(getExtensionPageUrl({ extensionId: ext.name })).toBe("/extension/foo-bar");
     });
 
-    it("returns matching page", () => {
-      const page = GlobalPageRegistry.getInstance().getByPageTarget({
-        pageId: "test-page",
-        extensionId: ext.name
-      });
-
-      expect(page.id).toEqual("test-page");
+    it("allows to pass base url as parameter", () => {
+      expect(getExtensionPageUrl({ extensionId: ext.name, pageId: "/test" })).toBe("/extension/foo-bar/test");
     });
 
-    it("returns null if target not found", () => {
-      const page = GlobalPageRegistry.getInstance().getByPageTarget({
-        pageId: "wrong-page",
-        extensionId: ext.name
+    it("removes @ and replace `/` to `--`", () => {
+      expect(getExtensionPageUrl({ extensionId: "@foo/bar" })).toBe("/extension/foo--bar");
+    });
+
+    it("adds / prefix", () => {
+      expect(getExtensionPageUrl({ extensionId: ext.name, pageId: "test" })).toBe("/extension/foo-bar/test");
+    });
+
+    it("normalize possible multi-slashes in page.id", () => {
+      expect(getExtensionPageUrl({ extensionId: ext.name, pageId: "//test/" })).toBe("/extension/foo-bar/test");
+    });
+
+    it("gets page url with custom params", () => {
+      const params: PageParams = { test1: "one", test2: "2" };
+      const searchParams = new URLSearchParams(params);
+      const pageUrl = getExtensionPageUrl({
+        extensionId: ext.name,
+        pageId: "page-with-params",
+        params,
       });
 
-      expect(page).toBeNull();
+      expect(pageUrl).toBe(`/extension/foo-bar/page-with-params?${searchParams}`);
+    });
+
+    it("gets page url with default custom params", () => {
+      const defaultPageUrl = getExtensionPageUrl({
+        extensionId: ext.name,
+        pageId: "page-with-params",
+      });
+
+      expect(defaultPageUrl).toBe(`/extension/foo-bar/page-with-params?test1=test1-default`);
+    });
+  });
+
+  describe("globalPageRegistry", () => {
+    describe("getByPageTarget", () => {
+      it("matching to first registered page without id", () => {
+        const page = GlobalPageRegistry.getInstance().getByPageTarget({ extensionId: ext.name });
+
+        expect(page.id).toEqual(undefined);
+        expect(page.extensionId).toEqual(ext.name);
+        expect(page.url).toEqual(getExtensionPageUrl({ extensionId: ext.name }));
+      });
+
+      it("returns matching page", () => {
+        const page = GlobalPageRegistry.getInstance().getByPageTarget({
+          pageId: "test-page",
+          extensionId: ext.name
+        });
+
+        expect(page.id).toEqual("test-page");
+      });
+
+      it("returns null if target not found", () => {
+        const page = GlobalPageRegistry.getInstance().getByPageTarget({
+          pageId: "wrong-page",
+          extensionId: ext.name
+        });
+
+        expect(page).toBeNull();
+      });
     });
   });
 });

src/extensions/renderer-api/components.ts

@@ -67,5 +67,5 @@ export * from "../../renderer/components/+events/kube-event-details";
 
 // specific exports
 export * from "../../renderer/components/status-brick";
-export { terminalStore, createTerminalTab } from "../../renderer/components/dock/terminal.store";
+export { terminalStore, createTerminalTab, TerminalStore } from "../../renderer/components/dock/terminal.store";
 export { logTabStore } from "../../renderer/components/dock/log-tab.store";

src/main/index.ts

@@ -62,7 +62,7 @@ import { FilesystemProvisionerStore } from "./extension-filesystem";
 import { SentryInit } from "../common/sentry";
 
 // This has to be called before start using winton-based logger
-// For example, before any logger.log 
+// For example, before any logger.log
 SentryInit();
 
 const workingDir = path.join(app.getPath("appData"), appName);
@@ -145,8 +145,12 @@ app.on("ready", async () => {
 
   UserStore.createInstance().startMainReactions();
 
+  // ClusterStore depends on: UserStore
   ClusterStore.createInstance().provideInitialFromMain();
+
+  // HotbarStore depends on: ClusterStore
   HotbarStore.createInstance();
+
   ExtensionsStore.createInstance();
   FilesystemProvisionerStore.createInstance();
   WeblinkStore.createInstance();

src/renderer/bootstrap.tsx

@@ -48,6 +48,7 @@ import { ExtensionsStore } from "../extensions/extensions-store";
 import { FilesystemProvisionerStore } from "../main/extension-filesystem";
 import { ThemeStore } from "./theme.store";
 import { SentryInit } from "../common/sentry";
+import { TerminalStore } from "./components/dock/terminal.store";
 
 configurePackages();
 
@@ -89,18 +90,28 @@ export async function bootstrap(App: AppComponent) {
 
   SentryInit();
 
-  await ClusterStore.createInstance().loadInitialOnRenderer();
+  // ClusterStore depends on: UserStore
+  const cs = ClusterStore.createInstance();
+
+  await cs.loadInitialOnRenderer();
+
+  // HotbarStore depends on: ClusterStore
   HotbarStore.createInstance();
   ExtensionsStore.createInstance();
   FilesystemProvisionerStore.createInstance();
+
+  // ThemeStore depends on: UserStore
   ThemeStore.createInstance();
+
+  // TerminalStore depends on: ThemeStore
+  TerminalStore.createInstance();
   WeblinkStore.createInstance();
 
   ExtensionInstallationStateStore.bindIpcListeners();
   HelmRepoManager.createInstance(); // initialize the manager
 
   // Register additional store listeners
-  ClusterStore.getInstance().registerIpcListener();
+  cs.registerIpcListener();
 
   // init app's dependencies if any
   if (App.init) {

src/renderer/components/dock/__test__/dock-tabs.test.tsx

@@ -22,14 +22,18 @@
 import React from "react";
 import { fireEvent, render } from "@testing-library/react";
 import "@testing-library/jest-dom/extend-expect";
+import fse from "fs-extra";
 
 import { DockTabs } from "../dock-tabs";
 import { dockStore, DockTab, TabKind } from "../dock.store";
 import { noop } from "../../../utils";
+import { ThemeStore } from "../../../theme.store";
+import { TerminalStore } from "../terminal.store";
+import { UserStore } from "../../../../common/user-store";
 
 jest.mock("electron", () => ({
   app: {
-    getPath: () => "/foo",
+    getPath: () => "tmp",
   },
 }));
 
@@ -55,10 +59,20 @@ const getTabKinds = () => dockStore.tabs.map(tab => tab.kind);
 
 describe("<DockTabs />", () => {
   beforeEach(async () => {
+    UserStore.createInstance();
+    ThemeStore.createInstance();
+    TerminalStore.createInstance();
     await dockStore.whenReady;
     dockStore.tabs = initialTabs;
   });
 
+  afterEach(() => {
+    ThemeStore.resetInstance();
+    TerminalStore.resetInstance();
+    UserStore.resetInstance();
+    fse.remove("tmp");
+  });
+
   it("renders w/o errors", () => {
     const { container } = renderTabs();
 

src/renderer/components/dock/__test__/log-tab.store.test.ts

@@ -20,14 +20,18 @@
  */
 
 import { podsStore } from "../../+workloads-pods/pods.store";
+import { UserStore } from "../../../../common/user-store";
 import { Pod } from "../../../api/endpoints";
+import { ThemeStore } from "../../../theme.store";
 import { dockStore } from "../dock.store";
 import { logTabStore } from "../log-tab.store";
+import { TerminalStore } from "../terminal.store";
 import { deploymentPod1, deploymentPod2, deploymentPod3, dockerPod } from "./pod.mock";
+import fse from "fs-extra";
 
 jest.mock("electron", () => ({
   app: {
-    getPath: () => "/foo",
+    getPath: () => "tmp",
   },
 }));
 
@@ -36,9 +40,19 @@ podsStore.items.push(new Pod(deploymentPod1));
 podsStore.items.push(new Pod(deploymentPod2));
 
 describe("log tab store", () => {
+  beforeEach(() => {
+    UserStore.createInstance();
+    ThemeStore.createInstance();
+    TerminalStore.createInstance();
+  });
+
   afterEach(() => {
     logTabStore.reset();
     dockStore.reset();
+    UserStore.resetInstance();
+    ThemeStore.resetInstance();
+    TerminalStore.resetInstance();
+    fse.remove("tmp");
   });
 
   it("creates log tab without sibling pods", () => {

src/renderer/components/dock/dock.store.ts

@@ -136,7 +136,12 @@ export class DockStore implements DockStorageState {
   }
 
   get selectedTabId(): TabId | undefined {
-    return this.storage.get().selectedTabId || this.tabs[0]?.id;
+    return this.storage.get().selectedTabId
+      || (
+        this.tabs.length > 0
+          ? this.tabs[0]?.id
+          : undefined
+      );
   }
 
   set selectedTabId(tabId: TabId) {
@@ -277,9 +282,9 @@ export class DockStore implements DockStorageState {
 
         if (newTab?.kind === TabKind.TERMINAL) {
           // close the dock when selected sibling inactive terminal tab
-          const { terminalStore } = await import("./terminal.store");
+          const { TerminalStore } = await import("./terminal.store");
 
-          if (!terminalStore.isConnected(newTab.id)) this.close();
+          if (!TerminalStore.getInstance(false)?.isConnected(newTab.id)) this.close();
         }
         this.selectTab(newTab.id);
       } else {

src/renderer/components/dock/log-tab.store.ts

@@ -108,8 +108,10 @@ export class LogTabStore extends DockTabStore<LogTabData> {
     });
   }
 
-  private updateTabsData() {
-    this.data.forEach((tabData, tabId) => {
+  private async updateTabsData() {
+    const promises: Promise<void>[] = [];
+
+    for (const [tabId, tabData] of this.data) {
       const pod = new Pod(tabData.selectedPod);
       const pods = podsStore.getPodsByOwnerId(pod.getOwnerRefs()[0]?.uid);
       const isSelectedPodInList = pods.find(item => item.getId() == pod.getId());
@@ -126,14 +128,16 @@ export class LogTabStore extends DockTabStore<LogTabData> {
 
         this.renameTab(tabId);
       } else {
-        this.closeTab(tabId);
+        promises.push(this.closeTab(tabId));
       }
-    });
+    }
+
+    await Promise.all(promises);
   }
 
-  private closeTab(tabId: string) {
+  private async closeTab(tabId: string) {
     this.clearData(tabId);
-    dockStore.closeTab(tabId);
+    await dockStore.closeTab(tabId);
   }
 }
 

src/renderer/components/dock/terminal.store.ts

@@ -20,7 +20,7 @@
  */
 
 import { autorun, observable } from "mobx";
-import { autoBind } from "../../utils";
+import { autoBind, Singleton } from "../../utils";
 import { Terminal } from "./terminal";
 import { TerminalApi } from "../../api/terminal-api";
 import { dockStore, DockTab, DockTabCreateSpecific, TabId, TabKind } from "./dock.store";
@@ -38,11 +38,12 @@ export function createTerminalTab(tabParams: DockTabCreateSpecific = {}) {
   });
 }
 
-export class TerminalStore {
+export class TerminalStore extends Singleton {
   protected terminals = new Map<TabId, Terminal>();
   protected connections = observable.map<TabId, TerminalApi>();
 
   constructor() {
+    super();
     autoBind(this);
 
     // connect active tab
@@ -129,4 +130,24 @@ export class TerminalStore {
   }
 }
 
-export const terminalStore = new TerminalStore();
+/**
+ * @deprecated use `TerminalStore.getInstance()` instead
+ */
+export const terminalStore = new Proxy({}, {
+  get(target, p) {
+    if (p === "$$typeof") {
+      return "TerminalStore";
+    }
+
+    const ts = TerminalStore.getInstance();
+    const res = (ts as any)?.[p];
+
+    if (typeof res === "function") {
+      return function(...args: any[]) {
+        return res.apply(ts, args);
+      };
+    }
+
+    return res;
+  },
+}) as TerminalStore;

src/renderer/components/tabs/tabs.tsx

@@ -84,7 +84,7 @@ export interface TabProps<D = any> extends DOMAttributes<HTMLElement> {
 export class Tab extends React.PureComponent<TabProps> {
   static contextType = TabsContext;
   declare context: TabsContextValue;
-  public elem: HTMLElement;
+  public ref = React.createRef<HTMLDivElement>();
 
   get isActive() {
     const { active, value } = this.props;
@@ -93,11 +93,11 @@ export class Tab extends React.PureComponent<TabProps> {
   }
 
   focus() {
-    this.elem.focus();
+    this.ref.current?.focus();
   }
 
   scrollIntoView() {
-    this.elem.scrollIntoView({
+    this.ref.current?.scrollIntoView?.({
       behavior: "smooth",
       inline: "center",
     });
@@ -106,30 +106,28 @@ export class Tab extends React.PureComponent<TabProps> {
   @boundMethod
   onClick(evt: React.MouseEvent<HTMLElement>) {
     const { value, active, disabled, onClick } = this.props;
-    const { onChange } = this.context;
 
-    if (disabled || active) return;
-    if (onClick) onClick(evt);
-    if (onChange) onChange(value);
+    if (disabled || active) {
+      return;
+    }
+
+    onClick?.(evt);
+    this.context.onChange?.(value);
   }
 
   @boundMethod
   onFocus(evt: React.FocusEvent<HTMLElement>) {
-    const { onFocus } = this.props;
-
-    if (onFocus) onFocus(evt);
+    this.props.onFocus?.(evt);
     this.scrollIntoView();
   }
 
   @boundMethod
   onKeyDown(evt: React.KeyboardEvent<HTMLElement>) {
-    const ENTER_KEY = evt.keyCode === 13;
-    const SPACE_KEY = evt.keyCode === 32;
+    if (evt.key === " " || evt.key === "Enter") {
+      this.ref.current?.click();
+    }
 
-    if (SPACE_KEY || ENTER_KEY) this.elem.click();
-    const { onKeyDown } = this.props;
-
-    if (onKeyDown) onKeyDown(evt);
+    this.props?.onKeyDown(evt);
   }
 
   componentDidMount() {
@@ -138,11 +136,6 @@ export class Tab extends React.PureComponent<TabProps> {
     }
   }
 
-  @boundMethod
-  protected bindRef(elem: HTMLElement) {
-    this.elem = elem;
-  }
-
   render() {
     const { active, disabled, icon, label, value, ...elemProps } = this.props;
     let { className } = this.props;
@@ -160,7 +153,7 @@ export class Tab extends React.PureComponent<TabProps> {
         onClick={this.onClick}
         onFocus={this.onFocus}
         onKeyDown={this.onKeyDown}
-        ref={this.bindRef}
+        ref={this.ref}
       >
         {typeof icon === "string" ? <Icon small material={icon}/> : icon}
         <div className="label">