Extensions api fixes (#1233)

* fix: create extension instance only when enabled

Signed-off-by: Roman <ixrock@gmail.com>

* mark extension.isEnabled with private modifier

Signed-off-by: Roman <ixrock@gmail.com>

* try-catch errors for extension.disable()

Signed-off-by: Roman <ixrock@gmail.com>

* fixes & refactoring

Signed-off-by: Roman <ixrock@gmail.com>

* make ext.isBundled non optional

Signed-off-by: Roman <ixrock@gmail.com>

src/extensions/extension-loader.ts

@@ -4,10 +4,11 @@ import type { LensRendererExtension } from "./lens-renderer-extension"
 import type { InstalledExtension } from "./extension-manager";
 import path from "path"
 import { broadcastIpc } from "../common/ipc"
-import { computed, observable, reaction, when } from "mobx"
+import { action, computed, observable, reaction, toJS, when } from "mobx"
 import logger from "../main/logger"
 import { app, ipcRenderer, remote } from "electron"
 import * as registries from "./registries";
+import { extensionsStore } from "./extensions-store";
 
 // lazy load so that we get correct userData
 export function extensionPackagesRoot() {
@@ -15,33 +16,42 @@ export function extensionPackagesRoot() {
 }
 
 export class ExtensionLoader {
+  protected extensions = observable.map<LensExtensionId, InstalledExtension>();
+  protected instances = observable.map<LensExtensionId, LensExtension>();
+
   @observable isLoaded = false;
-  protected extensions = observable.map<LensExtensionId, InstalledExtension>([], { deep: false });
+  whenLoaded = when(() => this.isLoaded);
-  protected instances = observable.map<LensExtensionId, LensExtension>([], { deep: false })
 
   constructor() {
     if (ipcRenderer) {
-      ipcRenderer.on("extensions:loaded", (event, extensions: InstalledExtension[]) => {
+      ipcRenderer.on("extensions:loaded", (event, extensions: [LensExtensionId, InstalledExtension][]) => {
         this.isLoaded = true;
-        extensions.forEach((ext) => {
+        extensions.forEach(([extId, ext]) => {
-          if (!this.extensions.has(ext.manifestPath)) {
+          if (!this.extensions.has(extId)) {
-            this.extensions.set(ext.manifestPath, ext)
+            this.extensions.set(extId, ext)
           }
         })
       });
     }
+    extensionsStore.manageState(this);
   }
 
-  @computed get userExtensions(): LensExtension[] {
+  @computed get userExtensions(): Map<LensExtensionId, InstalledExtension> {
-    return [...this.instances.values()].filter(ext => !ext.isBundled)
+    const extensions = this.extensions.toJS();
+    extensions.forEach((ext, extId) => {
+      if (ext.isBundled) {
+        extensions.delete(extId);
+      }
+    })
+    return extensions;
   }
 
-  async init() {
+  @action
-    const { extensionManager } = await import("./extension-manager");
+  async init(extensions: Map<LensExtensionId, InstalledExtension>) {
-    const installedExtensions = await extensionManager.load();
+    this.extensions.replace(extensions);
-    this.extensions.replace(installedExtensions);
     this.isLoaded = true;
     this.loadOnMain();
+    this.broadcastExtensions();
   }
 
   loadOnMain() {
@@ -71,21 +81,26 @@ export class ExtensionLoader {
   }
 
   protected autoInitExtensions(register: (ext: LensExtension) => Function[]) {
-    return reaction(() => this.extensions.toJS(), (installedExtensions) => {
+    return reaction(() => this.toJSON(), installedExtensions => {
-      for (const [id, ext] of installedExtensions) {
+      for (const [extId, ext] of installedExtensions) {
-        let instance = this.instances.get(ext.manifestPath)
+        let instance = this.instances.get(extId);
-        if (!instance) {
+        if (ext.isEnabled && !instance) {
-          const extensionModule = this.requireExtension(ext)
-          if (!extensionModule) {
-            continue
-          }
           try {
-            const LensExtensionClass: LensExtensionConstructor = extensionModule.default;
+            const LensExtensionClass: LensExtensionConstructor = this.requireExtension(ext)
+            if (!LensExtensionClass) continue;
             instance = new LensExtensionClass(ext);
             instance.whenEnabled(() => register(instance));
-            this.instances.set(ext.manifestPath, instance);
+            instance.enable();
+            this.instances.set(extId, instance);
           } catch (err) {
-            logger.error(`[EXTENSIONS-LOADER]: init extension instance error`, { ext, err })
+            logger.error(`[EXTENSION-LOADER]: activation extension error`, { ext, err })
+          }
+        } else if (!ext.isEnabled && instance) {
+          try {
+            instance.disable();
+            this.instances.delete(extId);
+          } catch (err) {
+            logger.error(`[EXTENSION-LOADER]: deactivation extension error`, { ext, err })
           }
         }
       }
@@ -103,7 +118,7 @@ export class ExtensionLoader {
         extEntrypoint = path.resolve(path.join(path.dirname(extension.manifestPath), extension.manifest.main))
       }
       if (extEntrypoint !== "") {
-        return __non_webpack_require__(extEntrypoint)
+        return __non_webpack_require__(extEntrypoint).default;
       }
     } catch (err) {
       console.error(`[EXTENSION-LOADER]: can't load extension main at ${extEntrypoint}: ${err}`, { extension });
@@ -111,6 +126,17 @@ export class ExtensionLoader {
     }
   }
 
+  getExtension(extId: LensExtensionId): InstalledExtension {
+    return this.extensions.get(extId);
+  }
+
+  toJSON(): Map<LensExtensionId, InstalledExtension> {
+    return toJS(this.extensions, {
+      exportMapsAsObjects: false,
+      recurseEverything: true,
+    })
+  }
+
   async broadcastExtensions(frameId?: number) {
     await when(() => this.isLoaded);
     broadcastIpc({
@@ -118,7 +144,7 @@ export class ExtensionLoader {
       frameId: frameId,
       frameOnly: !!frameId,
       args: [
-        Array.from(this.extensions.toJS().values())
+        Array.from(this.toJSON()),
       ],
     })
   }

src/extensions/extension-manager.ts

@@ -8,9 +8,10 @@ import { extensionPackagesRoot } from "./extension-loader"
 import { getBundledExtensions } from "../common/utils/app-version"
 
 export interface InstalledExtension {
-  manifest: LensExtensionManifest;
+  readonly manifest: LensExtensionManifest;
-  manifestPath: string;
+  readonly manifestPath: string;
-  isBundled?: boolean; // defined in package.json
+  readonly isBundled: boolean; // defined in project root's package.json
+  isEnabled: boolean;
 }
 
 type Dependencies = {
@@ -77,7 +78,7 @@ export class ExtensionManager {
     return await this.loadExtensions();
   }
 
-  protected async getByManifest(manifestPath: string): Promise<InstalledExtension> {
+  protected async getByManifest(manifestPath: string, { isBundled = false } = {}): Promise<InstalledExtension> {
     let manifestJson: LensExtensionManifest;
     try {
       fs.accessSync(manifestPath, fs.constants.F_OK); // check manifest file for existence
@@ -88,6 +89,8 @@ export class ExtensionManager {
       return {
         manifestPath: path.join(this.nodeModulesPath, manifestJson.name, "package.json"),
         manifest: manifestJson,
+        isBundled: isBundled,
+        isEnabled: isBundled,
       }
     } catch (err) {
       logger.error(`[EXTENSION-MANAGER]: can't install extension at ${manifestPath}: ${err}`, { manifestJson });
@@ -129,9 +132,8 @@ export class ExtensionManager {
       }
       const absPath = path.resolve(folderPath, fileName);
       const manifestPath = path.resolve(absPath, "package.json");
-      const ext = await this.getByManifest(manifestPath).catch(() => null)
+      const ext = await this.getByManifest(manifestPath, { isBundled: true }).catch(() => null)
       if (ext) {
-        ext.isBundled = true;
         extensions.push(ext)
       }
     }

src/extensions/extensions-store.ts

@@ -0,0 +1,77 @@
+import type { LensExtensionId } from "./lens-extension";
+import type { ExtensionLoader } from "./extension-loader";
+import { BaseStore } from "../common/base-store"
+import { action, observable, reaction, toJS } from "mobx";
+
+export interface LensExtensionsStoreModel {
+  extensions: Record<LensExtensionId, LensExtensionState>;
+}
+
+export interface LensExtensionState {
+  enabled?: boolean;
+}
+
+export class ExtensionsStore extends BaseStore<LensExtensionsStoreModel> {
+  constructor() {
+    super({
+      configName: "lens-extensions",
+    });
+  }
+
+  protected state = observable.map<LensExtensionId, LensExtensionState>();
+
+  protected getState(extensionLoader: ExtensionLoader) {
+    const state: Record<LensExtensionId, LensExtensionState> = {};
+    return Array.from(extensionLoader.userExtensions).reduce((state, [extId, ext]) => {
+      state[extId] = {
+        enabled: ext.isEnabled,
+      }
+      return state;
+    }, state)
+  }
+
+  async manageState(extensionLoader: ExtensionLoader) {
+    await extensionLoader.whenLoaded;
+    await this.whenLoaded;
+
+    // activate user-extensions when state is ready
+    extensionLoader.userExtensions.forEach((ext, extId) => {
+      ext.isEnabled = this.isEnabled(extId);
+    });
+
+    // apply state on changes from store
+    reaction(() => this.state.toJS(), extensionsState => {
+      extensionsState.forEach((state, extId) => {
+        const ext = extensionLoader.getExtension(extId);
+        if (ext && !ext.isBundled) {
+          ext.isEnabled = state.enabled;
+        }
+      })
+    })
+
+    // save state on change `extension.isEnabled`
+    reaction(() => this.getState(extensionLoader), extensionsState => {
+      this.state.merge(extensionsState)
+    })
+  }
+
+  isEnabled(extId: LensExtensionId) {
+    const state = this.state.get(extId);
+    return !state /* enabled by default */ || state.enabled;
+  }
+
+  @action
+  protected fromStore({ extensions }: LensExtensionsStoreModel) {
+    this.state.merge(extensions);
+  }
+
+  toJSON(): LensExtensionsStoreModel {
+    return toJS({
+      extensions: this.state.toJSON(),
+    }, {
+      recurseEverything: true
+    })
+  }
+}
+
+export const extensionsStore = new ExtensionsStore();

src/extensions/lens-extension.ts

@@ -1,7 +1,6 @@
 import type { InstalledExtension } from "./extension-manager";
-import { action, reaction } from "mobx";
+import { action, observable, reaction } from "mobx";
 import logger from "../main/logger";
-import { ExtensionStore } from "./extension-store";
 
 export type LensExtensionId = string; // path to manifest (package.json)
 export type LensExtensionConstructor = new (...args: ConstructorParameters<typeof LensExtension>) => LensExtension;
@@ -14,35 +13,17 @@ export interface LensExtensionManifest {
   renderer?: string; // path to %ext/dist/renderer.js
 }
 
-export interface LensExtensionStoreModel {
+export class LensExtension {
-  isEnabled: boolean;
-}
-
-export class LensExtension<S extends ExtensionStore<LensExtensionStoreModel> = any> {
-  protected store: S;
   readonly manifest: LensExtensionManifest;
   readonly manifestPath: string;
   readonly isBundled: boolean;
 
+  @observable private isEnabled = false;
+
   constructor({ manifest, manifestPath, isBundled }: InstalledExtension) {
     this.manifest = manifest
     this.manifestPath = manifestPath
     this.isBundled = !!isBundled
-    this.init();
-  }
-
-  protected async init(store: S = createBaseStore().getInstance()) {
-    this.store = store;
-    await this.store.loadExtension(this);
-    reaction(() => this.store.data.isEnabled, (isEnabled = true) => {
-      this.toggle(isEnabled); // handle activation & deactivation
-    }, {
-      fireImmediately: true
-    });
-  }
-
-  get isEnabled() {
-    return !!this.store.data.isEnabled;
   }
 
   get id(): LensExtensionId {
@@ -64,7 +45,7 @@ export class LensExtension<S extends ExtensionStore<LensExtensionStoreModel> = a
   @action
   async enable() {
     if (this.isEnabled) return;
-    this.store.data.isEnabled = true;
+    this.isEnabled = true;
     this.onActivate();
     logger.info(`[EXTENSION]: enabled ${this.name}@${this.version}`);
   }
@@ -72,7 +53,7 @@ export class LensExtension<S extends ExtensionStore<LensExtensionStoreModel> = a
   @action
   async disable() {
     if (!this.isEnabled) return;
-    this.store.data.isEnabled = false;
+    this.isEnabled = false;
     this.onDeactivate();
     logger.info(`[EXTENSION]: disabled ${this.name}@${this.version}`);
   }
@@ -114,13 +95,3 @@ export class LensExtension<S extends ExtensionStore<LensExtensionStoreModel> = a
     // mock
   }
 }
-
-function createBaseStore() {
-  return class extends ExtensionStore<LensExtensionStoreModel> {
-    constructor() {
-      super({
-        configName: "state"
-      });
-    }
-  }
-}

src/main/index.ts

@@ -21,6 +21,8 @@ import { userStore } from "../common/user-store";
 import { workspaceStore } from "../common/workspace-store";
 import { appEventBus } from "../common/event-bus"
 import { extensionLoader } from "../extensions/extension-loader";
+import { extensionManager } from "../extensions/extension-manager";
+import { extensionsStore } from "../extensions/extensions-store";
 
 const workingDir = path.join(app.getPath("appData"), appName);
 let proxyPort: number;
@@ -52,6 +54,7 @@ app.on("ready", async () => {
     userStore.load(),
     clusterStore.load(),
     workspaceStore.load(),
+    extensionsStore.load(),
   ]);
 
   // find free port
@@ -76,7 +79,7 @@ app.on("ready", async () => {
   }
 
   LensExtensionsApi.windowManager = windowManager = new WindowManager(proxyPort);
-  extensionLoader.init(); // call after windowManager to see splash earlier
+  extensionLoader.init(await extensionManager.load()); // call after windowManager to see splash earlier
 
   setTimeout(() => {
     appEventBus.emit({ name: "app", action: "start" })

src/renderer/bootstrap.tsx

@@ -4,6 +4,8 @@ import React from "react";
 import * as Mobx from "mobx"
 import * as MobxReact from "mobx-react"
 import * as LensExtensions from "../extensions/extension-api"
+import { App } from "./components/app";
+import { LensApp } from "./lens-app";
 import { render, unmountComponentAtNode } from "react-dom";
 import { isMac } from "../common/vars";
 import { userStore } from "../common/user-store";
@@ -11,8 +13,7 @@ import { workspaceStore } from "../common/workspace-store";
 import { clusterStore } from "../common/cluster-store";
 import { i18nStore } from "./i18n";
 import { themeStore } from "./theme.store";
-import { App } from "./components/app";
+import { extensionsStore } from "../extensions/extensions-store";
-import { LensApp } from "./lens-app";
 
 type AppComponent = React.ComponentType & {
   init?(): Promise<void>;
@@ -34,6 +35,7 @@ export async function bootstrap(App: AppComponent) {
     userStore.load(),
     workspaceStore.load(),
     clusterStore.load(),
+    extensionsStore.load(),
     i18nStore.init(),
     themeStore.init(),
   ]);

src/renderer/components/+extensions/extensions.scss

@@ -2,11 +2,17 @@
   --width: 100%;
   --max-width: auto;
 
+  .extension-list {
     .extension {
       --flex-gap: $padding / 3;
       padding: $padding $padding * 2;
       background: $colorVague;
       border-radius: $radius;
+
+      &:not(:first-of-type) {
+        margin-top: $padding * 2;
+      }
+    }
   }
 
   .extensions-path {

src/renderer/components/+extensions/extensions.tsx

@@ -19,7 +19,8 @@ export class Extensions extends React.Component {
 
   @computed get extensions() {
     const searchText = this.search.toLowerCase();
-    return extensionLoader.userExtensions.filter(({ name, description }) => {
+    return Array.from(extensionLoader.userExtensions.values()).filter(ext => {
+      const { name, description } = ext.manifest;
       return [
         name.toLowerCase().includes(searchText),
         description.toLowerCase().includes(searchText),
@@ -68,9 +69,10 @@ export class Extensions extends React.Component {
       )
     }
     return extensions.map(ext => {
-      const { id, name, description, isEnabled } = ext;
+      const { manifestPath: extId, isEnabled, manifest } = ext;
+      const { name, description } = manifest;
       return (
-        <div key={id} className="extension flex gaps align-center">
+        <div key={extId} className="extension flex gaps align-center">
           <div className="box grow flex column gaps">
             <div className="package">
               Name: <code className="name">{name}</code>
@@ -80,10 +82,10 @@ export class Extensions extends React.Component {
             </div>
           </div>
           {!isEnabled && (
-            <Button plain active onClick={() => ext.enable()}>Enable</Button>
+            <Button plain active onClick={() => ext.isEnabled = true}>Enable</Button>
           )}
           {isEnabled && (
-            <Button accent onClick={() => ext.disable()}>Disable</Button>
+            <Button accent onClick={() => ext.isEnabled = false}>Disable</Button>
           )}
         </div>
       )
@@ -102,7 +104,7 @@ export class Extensions extends React.Component {
             value={this.search}
             onChange={(value) => this.search = value}
           />
-          <div className="extension-list flex column gaps">
+          <div className="extension-list">
             {this.renderExtensions()}
           </div>
         </WizardLayout>