Improve documentation of Singleton functions, change to createInstance (#2585)

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

src/common/__tests__/cluster-store.test.ts

@@ -59,7 +59,7 @@ describe("empty config", () => {
 
     mockFs(mockOpts);
 
-    await ClusterStore.getInstanceOrCreate().load();
+    await ClusterStore.createInstance().load();
   });
 
   afterEach(() => {
@@ -177,7 +177,7 @@ describe("config with existing clusters", () => {
 
     mockFs(mockOpts);
 
-    return ClusterStore.getInstanceOrCreate().load();
+    return ClusterStore.createInstance().load();
   });
 
   afterEach(() => {
@@ -274,7 +274,7 @@ users:
 
     mockFs(mockOpts);
 
-    return ClusterStore.getInstanceOrCreate().load();
+    return ClusterStore.createInstance().load();
   });
 
   afterEach(() => {
@@ -316,7 +316,7 @@ describe("pre 2.0 config with an existing cluster", () => {
 
     mockFs(mockOpts);
 
-    return ClusterStore.getInstanceOrCreate().load();
+    return ClusterStore.createInstance().load();
   });
 
   afterEach(() => {
@@ -386,7 +386,7 @@ describe("pre 2.6.0 config with a cluster that has arrays in auth config", () =>
 
     mockFs(mockOpts);
 
-    return ClusterStore.getInstanceOrCreate().load();
+    return ClusterStore.createInstance().load();
   });
 
   afterEach(() => {
@@ -430,7 +430,7 @@ describe("pre 2.6.0 config with a cluster icon", () => {
 
     mockFs(mockOpts);
 
-    return ClusterStore.getInstanceOrCreate().load();
+    return ClusterStore.createInstance().load();
   });
 
   afterEach(() => {
@@ -469,7 +469,7 @@ describe("for a pre 2.7.0-beta.0 config without a workspace", () => {
 
     mockFs(mockOpts);
 
-    return ClusterStore.getInstanceOrCreate().load();
+    return ClusterStore.createInstance().load();
   });
 
   afterEach(() => {
@@ -505,7 +505,7 @@ describe("pre 3.6.0-beta.1 config with an existing cluster", () => {
 
     mockFs(mockOpts);
 
-    return ClusterStore.getInstanceOrCreate().load();
+    return ClusterStore.createInstance().load();
   });
 
   afterEach(() => {

src/common/__tests__/hotbar-store.test.ts

@@ -5,7 +5,7 @@ import { HotbarStore } from "../hotbar-store";
 describe("HotbarStore", () => {
   beforeEach(() => {
     ClusterStore.resetInstance();
-    ClusterStore.getInstanceOrCreate();
+    ClusterStore.createInstance();
 
     HotbarStore.resetInstance();
     mockFs({ tmp: { "lens-hotbar-store.json": "{}" } });
@@ -17,7 +17,7 @@ describe("HotbarStore", () => {
 
   describe("load", () => {
     it("loads one hotbar by default", () => {
-      HotbarStore.getInstanceOrCreate().load();
+      HotbarStore.createInstance().load();
       expect(HotbarStore.getInstance().hotbars.length).toEqual(1);
     });
   });

src/common/__tests__/user-store.test.ts

@@ -28,7 +28,7 @@ describe("user store tests", () => {
       UserStore.resetInstance();
       mockFs({ tmp: { "config.json": "{}", "kube_config": "{}" } });
 
-      (UserStore.getInstanceOrCreate() as any).refreshNewContexts = jest.fn(() => Promise.resolve());
+      (UserStore.createInstance() as any).refreshNewContexts = jest.fn(() => Promise.resolve());
 
       return UserStore.getInstance().load();
     });
@@ -102,7 +102,7 @@ describe("user store tests", () => {
         }
       });
 
-      return UserStore.getInstanceOrCreate().load();
+      return UserStore.createInstance().load();
     });
 
     afterEach(() => {

src/common/utils/singleton.ts

@@ -1,10 +1,3 @@
-/**
- * Narrowing class instances to the one.
- * Use "private" or "protected" modifier for constructor (when overriding) to disallow "new" usage.
- *
- * @example
- *  const usersStore: UsersStore = UsersStore.getInstance();
- */
 type StaticThis<T, R extends any[]> = { new(...args: R): T };
 
 export class Singleton {
@@ -13,12 +6,28 @@ export class Singleton {
 
   constructor() {
     if (Singleton.creating.length === 0) {
-      throw new TypeError("A singleton class must be created by getInstanceOrCreate()");
+      throw new TypeError("A singleton class must be created by createInstance()");
     }
   }
 
-  static getInstanceOrCreate<T, R extends any[]>(this: StaticThis<T, R>, ...args: R): T {
+  /**
+   * Creates the single instance of the child class if one was not already created.
+   *
+   * Multiple calls will return the same instance.
+   * Essentially throwing away the arguments to the subsequent calls.
+   *
+   * Note: this is a racy function, if two (or more) calls are racing to call this function
+   * only the first's arguments will be used.
+   * @param this Implicit argument that is the child class type
+   * @param args The constructor arguments for the child class
+   * @returns An instance of the child class
+   */
+  static createInstance<T, R extends any[]>(this: StaticThis<T, R>, ...args: R): T {
     if (!Singleton.instances.has(this)) {
+      if (Singleton.creating.length > 0) {
+        throw new TypeError("Cannot create a second singleton while creating a first");
+      }
+
       Singleton.creating = this.name;
       Singleton.instances.set(this, new this(...args));
       Singleton.creating = "";
@@ -27,6 +36,13 @@ export class Singleton {
     return Singleton.instances.get(this) as T;
   }
 
+  /**
+   * Get the instance of the child class that was previously created.
+   * @param this Implicit argument that is the child class type
+   * @param strict If false will return `undefined` instead of throwing when an instance doesn't exist.
+   * Default: `true`
+   * @returns An instance of the child class
+   */
   static getInstance<T, R extends any[]>(this: StaticThis<T, R>, strict = true): T | undefined {
     if (!Singleton.instances.has(this) && strict) {
       throw new TypeError(`instance of ${this.name} is not created`);
@@ -35,6 +51,13 @@ export class Singleton {
     return Singleton.instances.get(this) as (T | undefined);
   }
 
+  /**
+   * Delete the instance of the child class.
+   *
+   * Note: this doesn't prevent callers of `getInstance` from storing the result in a global.
+   *
+   * There is *no* way in JS or TS to prevent globals like that.
+   */
   static resetInstance() {
     Singleton.instances.delete(this);
   }