Fix some edge cases

- Clear the loading state in more places
- Add loading animation when an extension is discovered by main

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

src/extensions/extension-discovery.ts

@@ -1,13 +1,14 @@
 import { watch } from "chokidar";
 import { ipcRenderer } from "electron";
 import { EventEmitter } from "events";
-import fs from "fs-extra";
+import fse from "fs-extra";
 import { observable, reaction, toJS, when } from "mobx";
 import os from "os";
 import path from "path";
 import { broadcastMessage, handleRequest, requestMain, subscribeToBroadcast } from "../common/ipc";
 import { getBundledExtensions } from "../common/utils/app-version";
 import logger from "../main/logger";
+import { ExtensionInstallationStateStore } from "../renderer/components/+extensions/extension-install.store";
 import { extensionInstaller, PackageJson } from "./extension-installer";
 import { extensionLoader } from "./extension-loader";
 import { extensionsStore } from "./extensions-store";
@@ -40,7 +41,7 @@ interface ExtensionDiscoveryChannelMessage {
  * Returns true if the lstat is for a directory-like file (e.g. isDirectory or symbolic link)
  * @param lstat the stats to compare
  */
-const isDirectoryLike = (lstat: fs.Stats) => lstat.isDirectory() || lstat.isSymbolicLink();
+const isDirectoryLike = (lstat: fse.Stats) => lstat.isDirectory() || lstat.isSymbolicLink();
 
 /**
  * Discovers installed bundled and local extensions from the filesystem.
@@ -137,7 +138,7 @@ export class ExtensionDiscovery {
       depth: 1,
       ignoreInitial: true,
       // Try to wait until the file has been completely copied.
-      // The OS might emit an event for added file even it's not completely written to the filesysten.
+      // The OS might emit an event for added file even it's not completely written to the filesystem.
       awaitWriteFinish: {
         // Wait 300ms until the file size doesn't change to consider the file written.
         // For a small file like package.json this should be plenty of time.
@@ -146,8 +147,10 @@ export class ExtensionDiscovery {
     })
       // Extension add is detected by watching "<extensionDir>/package.json" add
       .on("add", this.handleWatchFileAdd)
-      // Extension remove is detected by watching <extensionDir>" unlink
+      // Extension remove is detected by watching "<extensionDir>" unlink
-      .on("unlinkDir", this.handleWatchUnlinkDir);
+      .on("unlinkDir", this.handleWatchUnlinkDir)
+      // Extension remove is detected by watching "<extensionSymLink>" unlink
+      .on("unlink", this.handleWatchUnlinkDir);
   }
 
   handleWatchFileAdd =  async (manifestPath: string) => {
@@ -161,6 +164,7 @@ export class ExtensionDiscovery {
 
     if (path.basename(manifestPath) === manifestFilename && isUnderLocalFolderPath) {
       try {
+        ExtensionInstallationStateStore.setInstallingFromMain(manifestPath);
         const absPath = path.dirname(manifestPath);
 
         // this.loadExtensionFromPath updates this.packagesJson
@@ -178,7 +182,9 @@ export class ExtensionDiscovery {
           this.events.emit("add", extension);
         }
       } catch (error) {
-        console.error(error);
+        logger.error(`${logModule}: failed to add extension: ${error}`, { error });
+      } finally {
+        ExtensionInstallationStateStore.clearInstallingFromMain(manifestPath);
       }
     }
   };
@@ -191,8 +197,9 @@ export class ExtensionDiscovery {
     // Check that the removed path is directly under this.localFolderPath
     // Note that the watcher can create unlink events for subdirectories of the extension
     const extensionFolderName = path.basename(filePath);
+    const expectedPath = path.relative(this.localFolderPath, filePath);
 
-    if (path.relative(this.localFolderPath, filePath) === extensionFolderName) {
+    if (expectedPath === extensionFolderName) {
       const extension = Array.from(this.extensions.values()).find((extension) => extension.absolutePath === filePath);
 
       if (extension) {
@@ -221,7 +228,7 @@ export class ExtensionDiscovery {
    * @param name e.g. "@mirantis/lens-extension-cc"
    */
   removeSymlinkByPackageName(name: string) {
-    return fs.remove(this.getInstalledPath(name));
+    return fse.remove(this.getInstalledPath(name));
   }
 
   /**
@@ -247,7 +254,7 @@ export class ExtensionDiscovery {
     await this.removeSymlinkByPackageName(manifest.name);
 
     // fs.remove does nothing if the path doesn't exist anymore
-    await fs.remove(absolutePath);
+    await fse.remove(absolutePath);
   }
 
   async load(): Promise<Map<LensExtensionId, InstalledExtension>> {
@@ -261,12 +268,11 @@ export class ExtensionDiscovery {
     logger.info(`${logModule} loading extensions from ${extensionInstaller.extensionPackagesRoot}`);
 
     // fs.remove won't throw if path is missing
-    await fs.remove(path.join(extensionInstaller.extensionPackagesRoot, "package-lock.json"));
+    await fse.remove(path.join(extensionInstaller.extensionPackagesRoot, "package-lock.json"));
-
 
     try {
       // Verify write access to static/extensions, which is needed for symlinking
-      await fs.access(this.inTreeFolderPath, fs.constants.W_OK);
+      await fse.access(this.inTreeFolderPath, fse.constants.W_OK);
 
       // Set bundled folder path to static/extensions
       this.bundledFolderPath = this.inTreeFolderPath;
@@ -275,26 +281,26 @@ export class ExtensionDiscovery {
       // The error can happen if there is read-only rights to static/extensions, which would fail symlinking.
 
       // Remove e.g. /Users/<username>/Library/Application Support/LensDev/extensions
-      await fs.remove(this.inTreeTargetPath);
+      await fse.remove(this.inTreeTargetPath);
 
       // Create folder e.g. /Users/<username>/Library/Application Support/LensDev/extensions
-      await fs.ensureDir(this.inTreeTargetPath);
+      await fse.ensureDir(this.inTreeTargetPath);
 
       // Copy static/extensions to e.g. /Users/<username>/Library/Application Support/LensDev/extensions
-      await fs.copy(this.inTreeFolderPath, this.inTreeTargetPath);
+      await fse.copy(this.inTreeFolderPath, this.inTreeTargetPath);
 
       // Set bundled folder path to e.g. /Users/<username>/Library/Application Support/LensDev/extensions
       this.bundledFolderPath = this.inTreeTargetPath;
     }
 
-    await fs.ensureDir(this.nodeModulesPath);
+    await fse.ensureDir(this.nodeModulesPath);
-    await fs.ensureDir(this.localFolderPath);
+    await fse.ensureDir(this.localFolderPath);
 
-    const extensions = await this.ensureExtensions();
+    try {
-
+      return await this.ensureExtensions();
-    this.isLoaded = true;
+    } finally {
-
+      this.isLoaded = true;
-    return extensions;
+    }
   }
 
   /**
@@ -317,30 +323,22 @@ export class ExtensionDiscovery {
    * Returns InstalledExtension from path to package.json file.
    * Also updates this.packagesJson.
    */
-  protected async getByManifest(manifestPath: string, { isBundled = false }: {
+  protected async getByManifest(manifestPath: string, { isBundled = false } = {}): Promise<InstalledExtension | null> {
-    isBundled?: boolean;
-  } = {}): Promise<InstalledExtension | null> {
-    let manifestJson: LensExtensionManifest;
-
     try {
-      // check manifest file for existence
+      const manifest = await fse.readJson(manifestPath);
-      fs.accessSync(manifestPath, fs.constants.F_OK);
+      const installedManifestPath = this.getInstalledManifestPath(manifest.name);
-
-      manifestJson = __non_webpack_require__(manifestPath);
-      const installedManifestPath = this.getInstalledManifestPath(manifestJson.name);
-
       const isEnabled = isBundled || extensionsStore.isEnabled(installedManifestPath);
 
       return {
         id: installedManifestPath,
         absolutePath: path.dirname(manifestPath),
         manifestPath: installedManifestPath,
-        manifest: manifestJson,
+        manifest,
         isBundled,
         isEnabled
       };
     } catch (error) {
-      logger.error(`${logModule}: can't load extension manifest at ${manifestPath}: ${error}`, { manifestJson });
+      logger.error(`${logModule}: can't load extension manifest at ${manifestPath}: ${error}`);
 
       return null;
     }
@@ -354,7 +352,7 @@ export class ExtensionDiscovery {
     const userExtensions = await this.loadFromFolder(this.localFolderPath);
 
     for (const extension of userExtensions) {
-      if (await fs.pathExists(extension.manifestPath) === false) {
+      if (await fse.pathExists(extension.manifestPath) === false) {
         await this.installPackage(extension.absolutePath);
       }
     }
@@ -386,7 +384,7 @@ export class ExtensionDiscovery {
     const extensions: InstalledExtension[] = [];
     const folderPath = this.bundledFolderPath;
     const bundledExtensions = getBundledExtensions();
-    const paths = await fs.readdir(folderPath);
+    const paths = await fse.readdir(folderPath);
 
     for (const fileName of paths) {
       if (!bundledExtensions.includes(fileName)) {
@@ -408,7 +406,7 @@ export class ExtensionDiscovery {
   async loadFromFolder(folderPath: string): Promise<InstalledExtension[]> {
     const bundledExtensions = getBundledExtensions();
     const extensions: InstalledExtension[] = [];
-    const paths = await fs.readdir(folderPath);
+    const paths = await fse.readdir(folderPath);
 
     for (const fileName of paths) {
       // do not allow to override bundled extensions
@@ -418,11 +416,11 @@ export class ExtensionDiscovery {
 
       const absPath = path.resolve(folderPath, fileName);
 
-      if (!fs.existsSync(absPath)) {
+      if (!fse.existsSync(absPath)) {
         continue;
       }
 
-      const lstat = await fs.lstat(absPath);
+      const lstat = await fse.lstat(absPath);
 
       // skip non-directories
       if (!isDirectoryLike(lstat)) {

src/extensions/extension-loader.ts

@@ -12,8 +12,6 @@ import type { LensExtension, LensExtensionConstructor, LensExtensionId } from ".
 import type { LensMainExtension } from "./lens-main-extension";
 import type { LensRendererExtension } from "./lens-renderer-extension";
 import * as registries from "./registries";
-import fs from "fs";
-
 
 export function extensionPackagesRoot() {
   return path.join((app || remote.app).getPath("userData"));
@@ -290,28 +288,20 @@ export class ExtensionLoader {
     });
   }
 
-  protected requireExtension(extension: InstalledExtension): LensExtensionConstructor {
+  protected requireExtension(extension: InstalledExtension): LensExtensionConstructor | null {
-    let extEntrypoint = "";
+    const entryPointName = ipcRenderer ? "renderer" : "main";
+    const extensionEntryPointRelativePath = extension.manifest[entryPointName];
+
+    if (!extensionEntryPointRelativePath) {
+      return null;
+    }
+
+    const extensionEntryPointAbsolutePath = path.resolve(path.join(path.dirname(extension.manifestPath), extensionEntryPointRelativePath));
 
     try {
-      if (ipcRenderer && extension.manifest.renderer) {
+      return __non_webpack_require__(extensionEntryPointAbsolutePath).default;
-        extEntrypoint = path.resolve(path.join(path.dirname(extension.manifestPath), extension.manifest.renderer));
+    } catch (error) {
-      } else if (!ipcRenderer && extension.manifest.main) {
+      logger.error(`${logModule}: can't load extension main at ${extensionEntryPointAbsolutePath}: ${error}`, { extension, error });
-        extEntrypoint = path.resolve(path.join(path.dirname(extension.manifestPath), extension.manifest.main));
-      }
-
-      if (extEntrypoint !== "") {
-        if (!fs.existsSync(extEntrypoint)) {
-          console.log(`${logModule}: entrypoint ${extEntrypoint} not found, skipping ...`);
-
-          return;
-        }
-
-        return __non_webpack_require__(extEntrypoint).default;
-      }
-    } catch (err) {
-      console.error(`${logModule}: can't load extension main at ${extEntrypoint}: ${err}`, { extension });
-      console.trace(err);
     }
   }
 

src/renderer/bootstrap.tsx

@@ -19,6 +19,7 @@ import { filesystemProvisionerStore } from "../main/extension-filesystem";
 import { App } from "./components/app";
 import { LensApp } from "./lens-app";
 import { themeStore } from "./theme.store";
+import { ExtensionInstallationStateStore } from "./components/+extensions/extension-install.store";
 
 /**
  * If this is a development buid, wait a second to attach
@@ -50,6 +51,7 @@ export async function bootstrap(App: AppComponent) {
   await attachChromeDebugger();
   rootElem.classList.toggle("is-mac", isMac);
 
+  ExtensionInstallationStateStore.bindIpcListeners();
   extensionLoader.init();
   extensionDiscovery.init();
 

src/renderer/components/+extensions/extension-install.store.ts

@@ -2,6 +2,8 @@ import { action, computed, observable } from "mobx";
 import logger from "../../../main/logger";
 import { disposer, Disposer } from "../../utils";
 import * as uuid from "uuid";
+import { broadcastMessage } from "../../../common/ipc";
+import { ipcRenderer } from "electron";
 
 export enum ExtensionInstallationState {
   INSTALLING = "installing",
@@ -15,6 +17,19 @@ const uninstallingExtensions = observable.set<string>();
 const preInstallIds = observable.set<string>();
 
 export class ExtensionInstallationStateStore {
+  private static InstallingFromMainChannel = "extension-installation-state-store:install";
+  private static ClearInstallingFromMainChannel = "extension-installation-state-store:clear-install";
+
+  static bindIpcListeners() {
+    ipcRenderer
+      .on(ExtensionInstallationStateStore.InstallingFromMainChannel, (event, extId) => {
+        ExtensionInstallationStateStore.setInstalling(extId);
+      })
+      .on(ExtensionInstallationStateStore.ClearInstallingFromMainChannel, (event, extId) => {
+        ExtensionInstallationStateStore.clearInstalling(extId);
+      });
+  }
+
   @action static reset() {
     logger.warn(`${Prefix}: resetting, may throw errors`);
     installingExtensions.clear();
@@ -39,6 +54,22 @@ export class ExtensionInstallationStateStore {
     installingExtensions.add(extId);
   }
 
+  /**
+   * Broadcasts that an extension is being installed by the main process
+   * @param extId the ID of the extension
+   */
+  static setInstallingFromMain(extId: string): void {
+    broadcastMessage(ExtensionInstallationStateStore.InstallingFromMainChannel, extId);
+  }
+
+  /**
+   * Broadcasts that an extension is no longer being installed by the main process
+   * @param extId the ID of the extension
+   */
+  static clearInstallingFromMain(extId: string): void {
+    broadcastMessage(ExtensionInstallationStateStore.ClearInstallingFromMainChannel, extId);
+  }
+
   /**
    * Marks the start of a pre-install phase of an extension installation. The
    * part of the installation before the tarball has been unpacked and the ID
@@ -165,7 +196,7 @@ export class ExtensionInstallationStateStore {
   }
 
   /**
-   * The current number of extensions preinstallig
+   * The current number of extensions preinstalling
    */
   @computed static get preinstalling(): number {
     return preInstallIds.size;

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

@@ -59,7 +59,7 @@ async function uninstallExtension(extensionId: LensExtensionId, manifest: LensEx
     return true;
   } catch (error) {
     Notifications.error(
-      <p>Uninstalling extension <b>{displayName}</b> has failed: <em>{error?.message ?? ""}</em></p>
+      <p>Uninstalling extension <b>{displayName}</b> has failed: <em>{error?.message ?? error?.toString() ?? ""}</em></p>
     );
 
     return false;
@@ -174,7 +174,7 @@ async function createTempFilesAndValidate(request: InstallRequestPreloaded, { sh
   return null;
 }
 
-async function unpackExtension(request: InstallRequestValidated, disposeDownloading: Disposer) {
+async function unpackExtension(request: InstallRequestValidated, disposeDownloading?: Disposer) {
   const { id, fileName, tempFile, manifest: { name, version } } = request;
 
   ExtensionInstallationStateStore.setInstalling(id);
@@ -238,13 +238,13 @@ async function requestInstall(request: InstallRequest, d?: Disposer): Promise<vo
   const loadedRequest = await preloadExtension(request);
 
   if (!loadedRequest) {
-    return;
+    return dispose();
   }
 
   const validatedRequest = await createTempFilesAndValidate(loadedRequest);
 
   if (!validatedRequest) {
-    return;
+    return dispose();
   }
 
   const { name, version, description } = validatedRequest.manifest;
@@ -282,10 +282,18 @@ async function requestInstall(request: InstallRequest, d?: Disposer): Promise<vo
         <Button autoFocus label="Install" onClick={async () => {
           removeNotification();
 
-          await uninstallExtension(validatedRequest.id, validatedRequest.manifest)
+          if (await uninstallExtension(validatedRequest.id, validatedRequest.manifest)) {
-            && await unpackExtension(validatedRequest, dispose);
+            await unpackExtension(validatedRequest, dispose);
+          } else {
+            disposer();
+          }
         }} />
-      </div>
+      </div>,
+      {
+        onClose() {
+          disposer();
+        }
+      }
     );
   }
 }
@@ -456,10 +464,6 @@ export class Extensions extends React.Component {
     return (
       <>
         {...searchedForExtensions.map(this.renderExtension)}
-        {
-          ExtensionInstallationStateStore.anyPreInstallingOrInstalling
-          && <div className="spinner-wrapper"><Spinner /></div>
-        }
       </>
     );
   }
@@ -505,6 +509,7 @@ export class Extensions extends React.Component {
               primary
               label="Install"
               disabled={ExtensionInstallationStateStore.anyPreInstallingOrInstalling || !Extensions.installPathValidator.validate(installPath)}
+              waiting={ExtensionInstallationStateStore.anyPreInstallingOrInstalling}
               onClick={() => installFromUrlOrPath(this.installPath)}
             />
             <small className="hint">