From 6df8cacf3601a8ba3a1f01af8254adf6a2f7fece Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Mon, 29 Mar 2021 13:43:13 -0400 Subject: [PATCH] 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 --- src/extensions/extension-discovery.ts | 78 +++++++++---------- src/extensions/extension-loader.ts | 34 +++----- src/renderer/bootstrap.tsx | 2 + .../+extensions/extension-install.store.ts | 33 +++++++- .../components/+extensions/extensions.tsx | 27 ++++--- 5 files changed, 100 insertions(+), 74 deletions(-) diff --git a/src/extensions/extension-discovery.ts b/src/extensions/extension-discovery.ts index 227a9aa59b..db43a70459 100644 --- a/src/extensions/extension-discovery.ts +++ b/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 "/package.json" add .on("add", this.handleWatchFileAdd) - // Extension remove is detected by watching " unlink - .on("unlinkDir", this.handleWatchUnlinkDir); + // Extension remove is detected by watching "" unlink + .on("unlinkDir", this.handleWatchUnlinkDir) + // Extension remove is detected by watching "" 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> { @@ -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//Library/Application Support/LensDev/extensions - await fs.remove(this.inTreeTargetPath); + await fse.remove(this.inTreeTargetPath); // Create folder e.g. /Users//Library/Application Support/LensDev/extensions - await fs.ensureDir(this.inTreeTargetPath); + await fse.ensureDir(this.inTreeTargetPath); // Copy static/extensions to e.g. /Users//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//Library/Application Support/LensDev/extensions this.bundledFolderPath = this.inTreeTargetPath; } - await fs.ensureDir(this.nodeModulesPath); - await fs.ensureDir(this.localFolderPath); + await fse.ensureDir(this.nodeModulesPath); + await fse.ensureDir(this.localFolderPath); - const extensions = await this.ensureExtensions(); - - this.isLoaded = true; - - return extensions; + try { + return await this.ensureExtensions(); + } finally { + this.isLoaded = true; + } } /** @@ -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 }: { - isBundled?: boolean; - } = {}): Promise { - let manifestJson: LensExtensionManifest; - + protected async getByManifest(manifestPath: string, { isBundled = false } = {}): Promise { try { - // check manifest file for existence - fs.accessSync(manifestPath, fs.constants.F_OK); - - manifestJson = __non_webpack_require__(manifestPath); - const installedManifestPath = this.getInstalledManifestPath(manifestJson.name); - + const manifest = await fse.readJson(manifestPath); + const installedManifestPath = this.getInstalledManifestPath(manifest.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 { 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)) { diff --git a/src/extensions/extension-loader.ts b/src/extensions/extension-loader.ts index 3af9ad874e..2c4016d51b 100644 --- a/src/extensions/extension-loader.ts +++ b/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 { - let extEntrypoint = ""; + protected requireExtension(extension: InstalledExtension): LensExtensionConstructor | null { + 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) { - extEntrypoint = path.resolve(path.join(path.dirname(extension.manifestPath), extension.manifest.renderer)); - } else if (!ipcRenderer && extension.manifest.main) { - 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); + return __non_webpack_require__(extensionEntryPointAbsolutePath).default; + } catch (error) { + logger.error(`${logModule}: can't load extension main at ${extensionEntryPointAbsolutePath}: ${error}`, { extension, error }); } } diff --git a/src/renderer/bootstrap.tsx b/src/renderer/bootstrap.tsx index 0d412e257c..bb35b4ce28 100644 --- a/src/renderer/bootstrap.tsx +++ b/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(); diff --git a/src/renderer/components/+extensions/extension-install.store.ts b/src/renderer/components/+extensions/extension-install.store.ts index 943aa424b5..4871529eeb 100644 --- a/src/renderer/components/+extensions/extension-install.store.ts +++ b/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(); const preInstallIds = observable.set(); 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; diff --git a/src/renderer/components/+extensions/extensions.tsx b/src/renderer/components/+extensions/extensions.tsx index 63114c328e..abd99a76c5 100644 --- a/src/renderer/components/+extensions/extensions.tsx +++ b/src/renderer/components/+extensions/extensions.tsx @@ -59,7 +59,7 @@ async function uninstallExtension(extensionId: LensExtensionId, manifest: LensEx return true; } catch (error) { Notifications.error( -

Uninstalling extension {displayName} has failed: {error?.message ?? ""}

+

Uninstalling extension {displayName} has failed: {error?.message ?? error?.toString() ?? ""}

); 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 { removeNotification(); - await uninstallExtension(validatedRequest.id, validatedRequest.manifest) - && await unpackExtension(validatedRequest, dispose); + if (await uninstallExtension(validatedRequest.id, validatedRequest.manifest)) { + await unpackExtension(validatedRequest, dispose); + } else { + disposer(); + } }} /> - + , + { + onClose() { + disposer(); + } + } ); } } @@ -456,10 +464,6 @@ export class Extensions extends React.Component { return ( <> {...searchedForExtensions.map(this.renderExtension)} - { - ExtensionInstallationStateStore.anyPreInstallingOrInstalling - &&
- } ); } @@ -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)} />