Skip to content

Commit

Permalink
Refactor the Extensions settings page
Browse files Browse the repository at this point in the history
- Simplify the install logic by refactoring out the multi-install logic

- Revamp the ExtensionInstallStateStore to more strictly track the
  lifetime of an install or uninstall request

- Fix the install of an already installed extension just hanging
  visually

- Display a spinner more often for more visual feedback

Signed-off-by: Sebastian Malton <sebastian@malton.name>
  • Loading branch information
Nokel81 committed Mar 8, 2021
1 parent f287b8a commit ddae241
Show file tree
Hide file tree
Showing 6 changed files with 581 additions and 403 deletions.
18 changes: 18 additions & 0 deletions src/common/utils/disposer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export type Disposer = () => void;

interface Extendable<T> {
push(...vals: T[]): void;
}

export function disposer(...args: Disposer[]): Disposer & Extendable<Disposer> {
const res = () => {
args.forEach(dispose => dispose?.());
args.length = 0;
};

res.push = (...vals: Disposer[]) => {
args.push(...vals);
};

return res;
}
1 change: 1 addition & 0 deletions src/common/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ export * from "./downloadFile";
export * from "./escapeRegExp";
export * from "./tar";
export * from "./type-narrowing";
export * from "./disposer";
27 changes: 15 additions & 12 deletions src/extensions/extension-discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,24 @@ import { broadcastMessage, handleRequest, requestMain, subscribeToBroadcast } fr
import { getBundledExtensions } from "../common/utils/app-version";
import logger from "../main/logger";
import { extensionInstaller, PackageJson } from "./extension-installer";
import { extensionLoader } from "./extension-loader";
import { extensionsStore } from "./extensions-store";
import type { LensExtensionId, LensExtensionManifest } from "./lens-extension";

export interface InstalledExtension {
id: LensExtensionId;
id: LensExtensionId;

readonly manifest: LensExtensionManifest;
readonly manifest: LensExtensionManifest;

// Absolute path to the non-symlinked source folder,
// e.g. "/Users/user/.k8slens/extensions/helloworld"
readonly absolutePath: string;
// Absolute path to the non-symlinked source folder,
// e.g. "/Users/user/.k8slens/extensions/helloworld"
readonly absolutePath: string;

// Absolute to the symlinked package.json file
readonly manifestPath: string;
readonly isBundled: boolean; // defined in project root's package.json
isEnabled: boolean;
}
// Absolute to the symlinked package.json file
readonly manifestPath: string;
readonly isBundled: boolean; // defined in project root's package.json
isEnabled: boolean;
}

const logModule = "[EXTENSION-DISCOVERY]";

Expand Down Expand Up @@ -236,9 +237,11 @@ export class ExtensionDiscovery {
/**
* Uninstalls extension.
* The application will detect the folder unlink and remove the extension from the UI automatically.
* @param extension Extension to unistall.
* @param extensionId The ID of the extension to uninstall.
*/
async uninstallExtension({ absolutePath, manifest }: InstalledExtension) {
async uninstallExtension(extensionId: LensExtensionId) {
const { manifest, absolutePath } = this.extensions.get(extensionId) ?? extensionLoader.getExtension(extensionId);

logger.info(`${logModule} Uninstalling ${manifest.name}`);

await this.removeSymlinkByPackageName(manifest.name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import React from "react";
import { extensionDiscovery } from "../../../../extensions/extension-discovery";
import { ConfirmDialog } from "../../confirm-dialog";
import { Notifications } from "../../notifications";
import { ExtensionStateStore } from "../extension-install.store";
import { ExtensionInstallationStateStore } from "../extension-install.store";
import { Extensions } from "../extensions";

jest.mock("fs-extra");
Expand Down Expand Up @@ -54,7 +54,7 @@ jest.mock("../../notifications", () => ({

describe("Extensions", () => {
beforeEach(() => {
ExtensionStateStore.resetInstance();
ExtensionInstallationStateStore.reset();
});

it("disables uninstall and disable buttons while uninstalling", async () => {
Expand Down Expand Up @@ -122,7 +122,7 @@ describe("Extensions", () => {

extensionDiscovery.isLoaded = true;

waitFor(() =>
waitFor(() =>
expect(container.querySelector(".Spinner")).not.toBeInTheDocument()
);
});
Expand Down
192 changes: 183 additions & 9 deletions src/renderer/components/+extensions/extension-install.store.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,187 @@
import { observable } from "mobx";
import { autobind, Singleton } from "../../utils";
import { action, computed, observable } from "mobx";
import logger from "../../../main/logger";
import { disposer, Disposer } from "../../utils";
import * as uuid from "uuid";

interface ExtensionState {
displayName: string;
// Possible states the extension can be
state: "installing" | "uninstalling";
export enum ExtensionInstallationState {
INSTALLING = "installing",
UNINSTALLING = "uninstalling",
IDLE = "IDLE",
}

@autobind()
export class ExtensionStateStore extends Singleton {
extensionState = observable.map<string, ExtensionState>();
const Prefix = "[ExtensionInstallationStore]";
const installingExtensions = observable.set<string>();
const uninstallingExtensions = observable.set<string>();
const preInstallIds = observable.set<string>();

export class ExtensionInstallationStateStore {
@action static reset() {
logger.warn(`${Prefix}: resetting, may throw errors`);
installingExtensions.clear();
uninstallingExtensions.clear();
preInstallIds.clear();
}

/**
* Strictly transitions an extension from not installing to installing
* @param extId the ID of the extension
* @throws if state is not IDLE
*/
@action static setInstalling(extId: string): void {
logger.debug(`${Prefix}: trying to set ${extId} as installing`);

const curState = ExtensionInstallationStateStore.getInstallationState(extId);

if (curState !== ExtensionInstallationState.IDLE) {
throw new Error(`${Prefix}: cannot set ${extId} as installing. Is currently ${curState}.`);
}

installingExtensions.add(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
* determined.
* @returns a disposer which should be called to mark the end of the install phase
*/
@action static startPreInstall(): Disposer {
const preInstallStepId = uuid.v4();

logger.debug(`${Prefix}: starting a new preinstall phase: ${preInstallStepId}`);
preInstallIds.add(preInstallStepId);

return disposer(() => {
preInstallIds.delete(preInstallStepId);
logger.debug(`${Prefix}: ending a preinstall phase: ${preInstallStepId}`);
});
}

/**
* Strictly transitions an extension from not uninstalling to uninstalling
* @param extId the ID of the extension
* @throws if state is not IDLE
*/
@action static setUninstalling(extId: string): void {
logger.debug(`${Prefix}: trying to set ${extId} as uninstalling`);

const curState = ExtensionInstallationStateStore.getInstallationState(extId);

if (curState !== ExtensionInstallationState.IDLE) {
throw new Error(`${Prefix}: cannot set ${extId} as uninstalling. Is currently ${curState}.`);
}

uninstallingExtensions.add(extId);
}

/**
* Strictly clears the INSTALLING state of an extension
* @param extId The ID of the extension
* @throws if state is not INSTALLING
*/
@action static clearInstalling(extId: string): void {
logger.debug(`${Prefix}: trying to clear ${extId} as installing`);

const curState = ExtensionInstallationStateStore.getInstallationState(extId);

switch (curState) {
case ExtensionInstallationState.INSTALLING:
return void installingExtensions.delete(extId);
default:
throw new Error(`${Prefix}: cannot clear INSTALLING state for ${extId}, it is currently ${curState}`);
}
}

/**
* Strictly clears the UNINSTALLING state of an extension
* @param extId The ID of the extension
* @throws if state is not UNINSTALLING
*/
@action static clearUninstalling(extId: string): void {
logger.debug(`${Prefix}: trying to clear ${extId} as uninstalling`);

const curState = ExtensionInstallationStateStore.getInstallationState(extId);

switch (curState) {
case ExtensionInstallationState.UNINSTALLING:
return void uninstallingExtensions.delete(extId);
default:
throw new Error(`${Prefix}: cannot clear UNINSTALLING state for ${extId}, it is currently ${curState}`);
}
}

/**
* Returns the current state of the extension. IDLE is default value.
* @param extId The ID of the extension
*/
static getInstallationState(extId: string): ExtensionInstallationState {
if (installingExtensions.has(extId)) {
return ExtensionInstallationState.INSTALLING;
}

if (uninstallingExtensions.has(extId)) {
return ExtensionInstallationState.UNINSTALLING;
}

return ExtensionInstallationState.IDLE;
}

/**
* Returns true if the extension is currently INSTALLING
* @param extId The ID of the extension
*/
static isExtensionInstalling(extId: string): boolean {
return ExtensionInstallationStateStore.getInstallationState(extId) === ExtensionInstallationState.INSTALLING;
}

/**
* Returns true if the extension is currently UNINSTALLING
* @param extId The ID of the extension
*/
static isExtensionUninstalling(extId: string): boolean {
return ExtensionInstallationStateStore.getInstallationState(extId) === ExtensionInstallationState.UNINSTALLING;
}

/**
* Returns true if the extension is currently IDLE
* @param extId The ID of the extension
*/
static isExtensionIdle(extId: string): boolean {
return ExtensionInstallationStateStore.getInstallationState(extId) === ExtensionInstallationState.IDLE;
}

/**
* The current number of extensions installing
*/
@computed static get installing(): number {
return installingExtensions.size;
}

/**
* If there is at least one extension currently installing
*/
@computed static get anyInstalling(): boolean {
return ExtensionInstallationStateStore.installing > 0;
}

/**
* The current number of extensions preinstallig
*/
@computed static get preinstalling(): number {
return preInstallIds.size;
}

/**
* If there is at least one extension currently downloading
*/
@computed static get anyPreinstalling(): boolean {
return ExtensionInstallationStateStore.preinstalling > 0;
}

/**
* If there is at least one installing or preinstalling step taking place
*/
@computed static get anyPreInstallingOrInstalling(): boolean {
return ExtensionInstallationStateStore.anyInstalling || ExtensionInstallationStateStore.anyPreinstalling;
}
}

0 comments on commit ddae241

Please sign in to comment.