Introduce clearer boundry between extensions (#7164)

- Bundled extensions are always enabled, and are always compatible
- Have bundled extensions be loaded asyncronously to support
  typescript dynamic import (which is typed) as opposed to require

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

packages/core/src/extensions/__tests__/extension-loader.test.ts

@@ -113,13 +113,13 @@ describe("ExtensionLoader", () => {
   });
 
   it("renderer updates extension after ipc broadcast", async () => {
-    expect(extensionLoader.userExtensions).toEqual(new Map());
+    expect(extensionLoader.userExtensions.get()).toEqual(new Map());
 
     await extensionLoader.init();
     await delay(10);
 
     // Assert the extensions after the extension broadcast event
-    expect(extensionLoader.userExtensions).toEqual(
+    expect(extensionLoader.userExtensions.get()).toEqual(
       new Map([
         ["manifest/path", {
           absolutePath: "/test/1",

packages/core/src/extensions/extension-discovery/extension-discovery.ts

@@ -10,7 +10,7 @@ import { broadcastMessage, ipcMainHandle, ipcRendererOn } from "../../common/ipc
 import { toJS } from "../../common/utils";
 import { isErrnoException } from "@k8slens/utilities";
 import type { ExtensionLoader } from "../extension-loader";
-import type { InstalledExtension, LensExtensionId, LensExtensionManifest } from "@k8slens/legacy-extensions";
+import type { InstalledExtension, LensExtensionId, LensExtensionManifest, ExternalInstalledExtension } from "@k8slens/legacy-extensions";
 import type { ExtensionInstallationStateStore } from "../extension-installation-state-store/extension-installation-state-store";
 import { extensionDiscoveryStateChannel } from "../../common/ipc/extension-handling";
 import { requestInitialExtensionDiscovery } from "../../renderer/ipc";
@@ -73,10 +73,6 @@ interface ExtensionDiscoveryChannelMessage {
  */
 const isDirectoryLike = (lstat: Stats) => lstat.isDirectory() || lstat.isSymbolicLink();
 
-interface LoadFromFolderOptions {
-  isBundled?: boolean;
-}
-
 interface ExtensionDiscoveryEvents {
   add: (ext: InstalledExtension) => void;
   remove: (extId: LensExtensionId) => void;
@@ -271,7 +267,7 @@ export class ExtensionDiscovery {
    * @param extensionId The ID of the extension to uninstall.
    */
   async uninstallExtension(extensionId: LensExtensionId): Promise<void> {
-    const extension = this.extensions.get(extensionId) ?? this.dependencies.extensionLoader.getExtension(extensionId);
+    const extension = this.extensions.get(extensionId) ?? this.dependencies.extensionLoader.getExtensionById(extensionId);
 
     if (!extension) {
       return void this.dependencies.logger.warn(`${logModule} could not uninstall extension, not found`, { id: extensionId });
@@ -330,24 +326,26 @@ export class ExtensionDiscovery {
    * Returns InstalledExtension from path to package.json file.
    * Also updates this.packagesJson.
    */
-  protected async getByManifest(manifestPath: string, { isBundled = false } = {}): Promise<InstalledExtension | null> {
+  protected async loadExtensionFromFolder(folderPath: string): Promise<ExternalInstalledExtension | null> {
+    const manifestPath = this.dependencies.joinPaths(folderPath, manifestFilename);
+
     try {
       const manifest = await this.dependencies.readJsonFile(manifestPath) as unknown as LensExtensionManifest;
-      const id = isBundled ? manifestPath : this.getInstalledManifestPath(manifest.name);
-      const isEnabled = this.dependencies.isExtensionEnabled({ id, isBundled });
+      const id = this.getInstalledManifestPath(manifest.name);
+      const isEnabled = this.dependencies.isExtensionEnabled(id);
       const extensionDir = this.dependencies.getDirnameOfPath(manifestPath);
       const npmPackage = this.dependencies.joinPaths(extensionDir, `${manifest.name}-${manifest.version}.tgz`);
       const absolutePath = this.dependencies.isProduction && await this.dependencies.pathExists(npmPackage)
         ? npmPackage
         : extensionDir;
-      const isCompatible = isBundled || this.dependencies.isCompatibleExtension(manifest);
+      const isCompatible = this.dependencies.isCompatibleExtension(manifest);
 
       return {
         id,
         absolutePath,
         manifestPath: id,
         manifest,
-        isBundled,
+        isBundled: false,
         isEnabled,
         isCompatible,
       };
@@ -363,14 +361,14 @@ export class ExtensionDiscovery {
     }
   }
 
-  async ensureExtensions(): Promise<Map<LensExtensionId, InstalledExtension>> {
+  async ensureExtensions(): Promise<Map<LensExtensionId, ExternalInstalledExtension>> {
     const userExtensions = await this.loadFromFolder(this.localFolderPath);
 
     return this.extensions = new Map(userExtensions.map(extension => [extension.id, extension]));
   }
 
-  async loadFromFolder(folderPath: string): Promise<InstalledExtension[]> {
-    const extensions: InstalledExtension[] = [];
+  async loadFromFolder(folderPath: string): Promise<ExternalInstalledExtension[]> {
+    const extensions: ExternalInstalledExtension[] = [];
     const paths = await this.dependencies.readDirectory(folderPath);
 
     for (const fileName of paths) {
@@ -403,16 +401,6 @@ export class ExtensionDiscovery {
     return extensions;
   }
 
-  /**
-   * Loads extension from absolute path, updates this.packagesJson to include it and returns the extension.
-   * @param folderPath Folder path to extension
-   */
-  async loadExtensionFromFolder(folderPath: string, { isBundled = false }: LoadFromFolderOptions = {}): Promise<InstalledExtension | null> {
-    const manifestPath = this.dependencies.joinPaths(folderPath, manifestFilename);
-
-    return this.getByManifest(manifestPath, { isBundled });
-  }
-
   toJSON(): ExtensionDiscoveryChannelMessage {
     return toJS({
       isLoaded: this.isLoaded,

packages/core/src/extensions/extension-loader/create-extension-instance.token.ts

@@ -3,11 +3,14 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 
-import type { LensExtensionConstructor, InstalledExtension } from "@k8slens/legacy-extensions";
+import type { LensExtensionConstructor, BundledInstalledExtension, ExternalInstalledExtension, BundledLensExtensionConstructor } from "@k8slens/legacy-extensions";
 import { getInjectionToken } from "@ogre-tools/injectable";
 import type { LensExtension } from "../lens-extension";
 
-export type CreateExtensionInstance = (ExtensionClass: LensExtensionConstructor, extension: InstalledExtension) => LensExtension;
+export interface CreateExtensionInstance {
+  (ExtensionClass: LensExtensionConstructor, extension: ExternalInstalledExtension): LensExtension;
+  (ExtensionClass: BundledLensExtensionConstructor, extension: BundledInstalledExtension): LensExtension;
+}
 
 export const createExtensionInstanceInjectionToken = getInjectionToken<CreateExtensionInstance>({
   id: "create-extension-instance-token",

packages/core/src/extensions/extension-loader/extension-loader.ts

@@ -6,9 +6,10 @@
 import { ipcMain, ipcRenderer } from "electron";
 import { isEqual } from "lodash";
 import type { ObservableMap } from "mobx";
-import { action, computed, makeObservable, toJS, observable, observe, reaction, when } from "mobx";
+import { runInAction, action, computed, toJS, observable, reaction, when } from "mobx";
 import { broadcastMessage, ipcMainOn, ipcRendererOn, ipcMainHandle } from "../../common/ipc";
-import { isDefined } from "@k8slens/utilities";
+import { isDefined, iter } from "@k8slens/utilities";
+import type { ExternalInstalledExtension, InstalledExtension, LensExtensionConstructor, LensExtensionId, BundledExtension } from "@k8slens/legacy-extensions";
 import type { LensExtension } from "../lens-extension";
 import { extensionLoaderFromMainChannel, extensionLoaderFromRendererChannel } from "../../common/ipc/extension-handling";
 import { requestExtensionLoaderInitialState } from "../../renderer/ipc";
@@ -19,7 +20,6 @@ import type { Extension } from "./extension/extension.injectable";
 import type { Logger } from "../../common/logger";
 import type { JoinPaths } from "../../common/path/join-paths.injectable";
 import type { GetDirnameOfPath } from "../../common/path/get-dirname.injectable";
-import type { LensExtensionId, BundledExtension, InstalledExtension, LensExtensionConstructor } from "@k8slens/legacy-extensions";
 import type { UpdateExtensionsState } from "../../features/extensions/enabled/common/update-state.injectable";
 
 const logModule = "[EXTENSIONS-LOADER]";
@@ -60,51 +60,21 @@ export class ExtensionLoader {
    */
   protected readonly nonInstancesByName = observable.set<string>();
 
-  /**
-   * This is updated by the `observe` in the constructor. DO NOT write directly to it
-   */
-  protected readonly instancesByName = observable.map<string, LensExtension>();
+  protected readonly instancesByName = computed(() => new Map((
+    iter.chain(this.dependencies.extensionInstances.entries())
+      .map(([, instance]) => [instance.name, instance])
+  )));
 
   private readonly onRemoveExtensionId = new EventEmitter<[string]>();
 
-  @observable isLoaded = false;
+  readonly isLoaded = observable.box(false);
 
-  get whenLoaded() {
-    return when(() => this.isLoaded);
-  }
+  constructor(protected readonly dependencies: Dependencies) {}
 
-  constructor(protected readonly dependencies: Dependencies) {
-    makeObservable(this);
-
-    observe(this.dependencies.extensionInstances, change => {
-      switch (change.type) {
-        case "add":
-          if (this.instancesByName.has(change.newValue.name)) {
-            throw new TypeError("Extension names must be unique");
-          }
-
-          this.instancesByName.set(change.newValue.name, change.newValue);
-          break;
-        case "delete":
-          this.instancesByName.delete(change.oldValue.name);
-          break;
-        case "update":
-          throw new Error("Extension instances shouldn't be updated");
-      }
-    });
-  }
-
-  @computed get userExtensions(): Map<LensExtensionId, InstalledExtension> {
-    const extensions = this.toJSON();
-
-    extensions.forEach((ext, extId) => {
-      if (ext.isBundled) {
-        extensions.delete(extId);
-      }
-    });
-
-    return extensions;
-  }
+  readonly userExtensions = computed(() => new Map((
+    this.extensions.toJSON()
+      .filter(([, extension]) => !extension.isBundled)
+  )));
 
   /**
    * Get the extension instance by its manifest name
@@ -120,19 +90,18 @@ export class ExtensionLoader {
       return null;
     }
 
-    return this.instancesByName.get(name);
+    return this.instancesByName.get().get(name);
   }
 
   // Transform userExtensions to a state object for storing into ExtensionsStore
-  @computed get storeState() {
-    return Array.from(this.userExtensions)
-      .map(([extId, extension]) => [extId, {
-        enabled: extension.isEnabled,
-        name: extension.manifest.name,
-      }] as const);
-  }
+  readonly storeState = computed(() => Array.from(
+    this.userExtensions.get(),
+    ([extId, extension]) => [extId, {
+      enabled: extension.isEnabled,
+      name: extension.manifest.name,
+    }] as const,
+  ));
 
-  @action
   async init() {
     if (ipcMain) {
       await this.initMain();
@@ -140,7 +109,7 @@ export class ExtensionLoader {
       await this.initRenderer();
     }
 
-    await Promise.all([this.whenLoaded]);
+    await when(() => this.isLoaded.get());
 
     // broadcasting extensions between main/renderer processes
     reaction(() => this.toJSON(), () => this.broadcastExtensions(), {
@@ -148,8 +117,7 @@ export class ExtensionLoader {
     });
 
     reaction(
-      () => this.storeState,
-
+      () => this.storeState.get(),
       (state) => {
         this.dependencies.updateExtensionsState(state);
       },
@@ -199,18 +167,20 @@ export class ExtensionLoader {
   setIsEnabled(lensExtensionId: LensExtensionId, isEnabled: boolean) {
     const extension = this.extensions.get(lensExtensionId);
 
-    assert(extension, `Must register extension ${lensExtensionId} with before enabling it`);
+    assert(extension, `Extension "${lensExtensionId}" must be registered before it can be enabled.`);
+    assert(!extension.isBundled, `Cannot change the enabled state of a bundled extension`);
 
     extension.isEnabled = isEnabled;
   }
 
   protected async initMain() {
-    this.isLoaded = true;
+    runInAction(() => {
+      this.isLoaded.set(true);
+    });
+
     await this.autoInitExtensions();
 
-    ipcMainHandle(extensionLoaderFromMainChannel, () => {
-      return Array.from(this.toJSON());
-    });
+    ipcMainHandle(extensionLoaderFromMainChannel, () => [...this.toJSON()]);
 
     ipcMainOn(extensionLoaderFromRendererChannel, (event, extensions: [LensExtensionId, InstalledExtension][]) => {
       this.syncExtensions(extensions);
@@ -219,7 +189,9 @@ export class ExtensionLoader {
 
   protected async initRenderer() {
     const extensionListHandler = (extensions: [LensExtensionId, InstalledExtension][]) => {
-      this.isLoaded = true;
+      runInAction(() => {
+        this.isLoaded.set(true);
+      });
       this.syncExtensions(extensions);
 
       const receivedExtensionIds = extensions.map(([lensExtensionId]) => lensExtensionId);
@@ -255,10 +227,10 @@ export class ExtensionLoader {
   }
 
   protected async loadBundledExtensions() {
-    return this.dependencies.bundledExtensions
-      .map(extension => {
+    const bundledExtensions = await Promise.all((this.dependencies.bundledExtensions
+      .map(async extension => {
         try {
-          const LensExtensionClass = extension[this.dependencies.extensionEntryPointName]();
+          const LensExtensionClass = await extension[this.dependencies.extensionEntryPointName]();
 
           if (!LensExtensionClass) {
             return null;
@@ -291,7 +263,9 @@ export class ExtensionLoader {
           return null;
         }
       })
-      .filter(isDefined);
+    ));
+
+    return bundledExtensions.filter(isDefined);
   }
 
   protected async loadExtensions(extensions: ExtensionBeingActivated[]): Promise<ExtensionLoading[]> {
@@ -332,6 +306,7 @@ export class ExtensionLoader {
     // 4. Return ExtensionLoading[]
 
     return [...installedExtensions.entries()]
+      .filter((entry): entry is [string, ExternalInstalledExtension] => !entry[1].isBundled)
       .map(([extId, extension]) => {
         const alreadyInit = this.dependencies.extensionInstances.has(extId) || this.nonInstancesByName.has(extension.manifest.name);
 
@@ -391,7 +366,7 @@ export class ExtensionLoader {
     return loadedExtensions;
   }
 
-  protected requireExtension(extension: InstalledExtension): LensExtensionConstructor | null {
+  protected requireExtension(extension: ExternalInstalledExtension): LensExtensionConstructor | null {
     const extRelativePath = extension.manifest[this.dependencies.extensionEntryPointName];
 
     if (!extRelativePath) {
@@ -411,7 +386,7 @@ export class ExtensionLoader {
     return null;
   }
 
-  getExtension(extId: LensExtensionId) {
+  getExtensionById(extId: LensExtensionId) {
     return this.extensions.get(extId);
   }
 

packages/core/src/extensions/lens-extension.ts

@@ -9,7 +9,6 @@ import type { LensExtensionDependencies } from "./lens-extension-set-dependencie
 import type { ProtocolHandlerRegistration } from "../common/protocol-handler/registration";
 import type { InstalledExtension, LegacyLensExtension, LensExtensionId, LensExtensionManifest } from "@k8slens/legacy-extensions";
 
-
 export const lensExtensionDependencies = Symbol("lens-extension-dependencies");
 export const Disposers = Symbol("disposers");
 
@@ -42,14 +41,12 @@ export class LensExtension<
   [Disposers] = disposer();
 
   constructor({ id, manifest, manifestPath, isBundled }: InstalledExtension) {
-    makeObservable(this);
-
     // id is the name of the manifest
     this.id = id;
-
-    this.manifest = manifest;
+    this.manifest = manifest as LensExtensionManifest;
     this.manifestPath = manifestPath;
     this.isBundled = !!isBundled;
+    makeObservable(this);
   }
 
   get name() {