diff --git a/package.json b/package.json index db8c3fcaac..889ca2cc9c 100644 --- a/package.json +++ b/package.json @@ -232,7 +232,7 @@ "mac-ca": "^1.0.4", "marked": "^1.1.0", "md5-file": "^5.0.0", - "mobx": "^5.15.5", + "mobx": "^5.15.7", "mobx-observable-history": "^1.0.3", "mock-fs": "^4.12.0", "node-pty": "^0.9.0", diff --git a/src/extensions/__tests__/extension-loader.test.ts b/src/extensions/__tests__/extension-loader.test.ts index 90aebfaee9..dd09d4614e 100644 --- a/src/extensions/__tests__/extension-loader.test.ts +++ b/src/extensions/__tests__/extension-loader.test.ts @@ -18,6 +18,7 @@ jest.mock( name: "TestExtension", version: "1.0.0", }, + id: manifestPath, absolutePath: "/test/1", manifestPath, isBundled: false, @@ -31,6 +32,7 @@ jest.mock( name: "TestExtension2", version: "2.0.0", }, + id: manifestPath2, absolutePath: "/test/2", manifestPath: manifestPath2, isBundled: false, @@ -54,6 +56,7 @@ jest.mock( name: "TestExtension", version: "1.0.0", }, + id: manifestPath, absolutePath: "/test/1", manifestPath, isBundled: false, @@ -67,6 +70,7 @@ jest.mock( name: "TestExtension3", version: "3.0.0", }, + id: manifestPath3, absolutePath: "/test/3", manifestPath: manifestPath3, isBundled: false, @@ -99,6 +103,7 @@ describe("ExtensionLoader", () => { Map { "manifest/path" => Object { "absolutePath": "/test/1", + "id": "manifest/path", "isBundled": false, "isEnabled": true, "manifest": Object { @@ -109,6 +114,7 @@ describe("ExtensionLoader", () => { }, "manifest/path3" => Object { "absolutePath": "/test/3", + "id": "manifest/path3", "isBundled": false, "isEnabled": true, "manifest": Object { diff --git a/src/extensions/__tests__/lens-extension.test.ts b/src/extensions/__tests__/lens-extension.test.ts index 277a76b410..897c246674 100644 --- a/src/extensions/__tests__/lens-extension.test.ts +++ b/src/extensions/__tests__/lens-extension.test.ts @@ -9,6 +9,7 @@ describe("lens extension", () => { name: "foo-bar", version: "0.1.1" }, + id: "/this/is/fake/package.json", absolutePath: "/absolute/fake/", manifestPath: "/this/is/fake/package.json", isBundled: false, diff --git a/src/extensions/extension-discovery.ts b/src/extensions/extension-discovery.ts index 452bba4d65..c249496fa6 100644 --- a/src/extensions/extension-discovery.ts +++ b/src/extensions/extension-discovery.ts @@ -10,6 +10,8 @@ import { extensionsStore } from "./extensions-store"; import type { LensExtensionId, LensExtensionManifest } from "./lens-extension"; export interface InstalledExtension { + id: LensExtensionId; + readonly manifest: LensExtensionManifest; // Absolute path to the non-symlinked source folder, @@ -254,6 +256,7 @@ export class ExtensionDiscovery { const isEnabled = isBundled || extensionsStore.isEnabled(installedManifestPath); return { + id: installedManifestPath, absolutePath: path.dirname(manifestPath), manifestPath: installedManifestPath, manifest: manifestJson, @@ -273,7 +276,7 @@ export class ExtensionDiscovery { await this.installPackages(); const extensions = bundledExtensions.concat(localExtensions); - return new Map(extensions.map(ext => [ext.manifestPath, ext])); + return new Map(extensions.map(extension => [extension.id, extension])); } /** diff --git a/src/extensions/extension-loader.ts b/src/extensions/extension-loader.ts index a73f173e7c..eb49021391 100644 --- a/src/extensions/extension-loader.ts +++ b/src/extensions/extension-loader.ts @@ -61,7 +61,7 @@ export class ExtensionLoader { } addExtension(extension: InstalledExtension) { - this.extensions.set(extension.manifestPath as LensExtensionId, extension); + this.extensions.set(extension.id, extension); } removeInstance(lensExtensionId: LensExtensionId) { @@ -139,8 +139,7 @@ export class ExtensionLoader { ]; this.events.on("remove", (removedExtension: LensRendererExtension) => { - // manifestPath is considered the id - if (removedExtension.manifestPath === extension.manifestPath) { + if (removedExtension.id === extension.id) { removeItems.forEach(remove => { remove(); }); @@ -163,7 +162,7 @@ export class ExtensionLoader { ]; this.events.on("remove", (removedExtension: LensRendererExtension) => { - if (removedExtension.manifestPath === extension.manifestPath) { + if (removedExtension.id === extension.id) { removeItems.forEach(remove => { remove(); }); @@ -191,7 +190,7 @@ export class ExtensionLoader { ]; this.events.on("remove", (removedExtension: LensRendererExtension) => { - if (removedExtension.manifestPath === extension.manifestPath) { + if (removedExtension.id === extension.id) { removeItems.forEach(remove => { remove(); }); diff --git a/src/extensions/lens-extension.ts b/src/extensions/lens-extension.ts index 1d16183d75..61dc6c0560 100644 --- a/src/extensions/lens-extension.ts +++ b/src/extensions/lens-extension.ts @@ -16,23 +16,20 @@ export interface LensExtensionManifest { } export class LensExtension { + readonly id: LensExtensionId; readonly manifest: LensExtensionManifest; readonly manifestPath: string; readonly isBundled: boolean; @observable private isEnabled = false; - constructor({ manifest, manifestPath, isBundled }: InstalledExtension) { + constructor({ id, manifest, manifestPath, isBundled }: InstalledExtension) { + this.id = id; this.manifest = manifest; this.manifestPath = manifestPath; this.isBundled = !!isBundled; } - get id(): LensExtensionId { - // This is the symlinked path under node_modules - return this.manifestPath; - } - get name() { return this.manifest.name; } diff --git a/src/extensions/registries/__tests__/page-registry.test.ts b/src/extensions/registries/__tests__/page-registry.test.ts index 4c94274a37..0467ae7ea4 100644 --- a/src/extensions/registries/__tests__/page-registry.test.ts +++ b/src/extensions/registries/__tests__/page-registry.test.ts @@ -11,6 +11,7 @@ describe("getPageUrl", () => { name: "foo-bar", version: "0.1.1" }, + id: "/this/is/fake/package.json", absolutePath: "/absolute/fake/", manifestPath: "/this/is/fake/package.json", isBundled: false, @@ -42,6 +43,7 @@ describe("globalPageRegistry", () => { name: "@acme/foo-bar", version: "0.1.1" }, + id: "/this/is/fake/package.json", absolutePath: "/absolute/fake/", manifestPath: "/this/is/fake/package.json", isBundled: false, diff --git a/src/jest.setup.ts b/src/jest.setup.ts index 7b4732930e..cccef274f0 100644 --- a/src/jest.setup.ts +++ b/src/jest.setup.ts @@ -1,4 +1,3 @@ - import fetchMock from "jest-fetch-mock"; // rewire global.fetch to call 'fetchMock' fetchMock.enableMocks(); diff --git a/src/renderer/components/+extensions/__tests__/extensions.test.tsx b/src/renderer/components/+extensions/__tests__/extensions.test.tsx new file mode 100644 index 0000000000..ce85139a76 --- /dev/null +++ b/src/renderer/components/+extensions/__tests__/extensions.test.tsx @@ -0,0 +1,44 @@ +import '@testing-library/jest-dom/extend-expect'; +import { fireEvent, render, screen } from '@testing-library/react'; +import React from 'react'; +import { extensionDiscovery } from "../../../../extensions/extension-discovery"; +import { Extensions } from "../extensions"; + +jest.mock("../../../../extensions/extension-discovery", () => ({ + ...jest.requireActual("../../../../extensions/extension-discovery"), + extensionDiscovery: { + localFolderPath: "/fake/path", + uninstallExtension: jest.fn() + } +})); + +jest.mock("../../../../extensions/extension-loader", () => ({ + ...jest.requireActual("../../../../extensions/extension-loader"), + extensionLoader: { + userExtensions: new Map([ + ["extensionId", { + id: "extensionId", + manifest: { + name: "test", + version: "1.2.3" + }, + absolutePath: "/absolute/path", + manifestPath: "/symlinked/path/package.json", + isBundled: false, + isEnabled: true + }] + ]) + } +})); + +describe("Extensions", () => { + it("disables uninstall and disable buttons while uninstalling", () => { + render(); + + fireEvent.click(screen.getByText("Uninstall")); + + expect(extensionDiscovery.uninstallExtension).toHaveBeenCalledWith("/absolute/path"); + expect(screen.getByText("Disable").closest("button")).toBeDisabled(); + expect(screen.getByText("Uninstall").closest("button")).toBeDisabled(); + }); +}); diff --git a/src/renderer/components/+extensions/extensions.tsx b/src/renderer/components/+extensions/extensions.tsx index 03f34f96ee..4b2a1d6dd7 100644 --- a/src/renderer/components/+extensions/extensions.tsx +++ b/src/renderer/components/+extensions/extensions.tsx @@ -1,8 +1,9 @@ import { t, Trans } from "@lingui/macro"; import { remote, shell } from "electron"; import fse from "fs-extra"; -import { computed, observable } from "mobx"; -import { observer } from "mobx-react"; +import { map, omit } from "lodash"; +import { computed, observable, ObservableMap, reaction } from "mobx"; +import { disposeOnUnmount, observer } from "mobx-react"; import os from "os"; import path from "path"; import React from "react"; @@ -38,6 +39,12 @@ interface InstallRequestValidated extends InstallRequestPreloaded { tempFile: string; // temp system path to packed extension for unpacking } +interface ExtensionState { + displayName: string; + // Possible states the extension can be + state: "uninstalling"; +} + @observer export class Extensions extends React.Component { private supportedFormats = [".tar", ".tgz"]; @@ -49,17 +56,47 @@ export class Extensions extends React.Component { } }; + @observable + extensionState = observable.map(); + @observable search = ""; @observable installPath = ""; + /** + * Extensions that were removed from extensions but are still in "uninstalling" state + */ + @computed get removedUninstalling() { + return Array.from(this.extensionState.entries()).filter(([id, extension]) => + extension.state === "uninstalling" && !this.extensions.find(extension => extension.id === id) + ).map(([id, extension]) => ({ ...extension, id })); + } + + componentDidMount() { + disposeOnUnmount(this, + reaction(() => this.extensions, (extensions) => { + const removedUninstalling = this.removedUninstalling; + + removedUninstalling.forEach(({ displayName }) => { + Notifications.ok( +

Extension {displayName} successfully uninstalled!

+ ); + }); + + removedUninstalling.forEach(({ id }) => { + this.extensionState.delete(id); + }); + }) + ); + } + @computed get extensions() { const searchText = this.search.toLowerCase(); return Array.from(extensionLoader.userExtensions.values()).filter(ext => { const { name, description } = ext.manifest; return [ name.toLowerCase().includes(searchText), - description.toLowerCase().includes(searchText), - ].some(v => v); + description?.toLowerCase().includes(searchText), + ].some(value => value); }); } @@ -278,14 +315,21 @@ export class Extensions extends React.Component { } async uninstallExtension(extension: InstalledExtension) { - const extensionName = extensionDisplayName(extension.manifest.name, extension.manifest.version); + const displayName = extensionDisplayName(extension.manifest.name, extension.manifest.version); try { + this.extensionState.set(extension.id, { + state: "uninstalling", + displayName + }); + await extensionDiscovery.uninstallExtension(extension.absolutePath); } catch (error) { Notifications.error( -

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

+

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

); + // Remove uninstall state on uninstall failure + this.extensionState.delete(extension.id); } } @@ -305,11 +349,12 @@ export class Extensions extends React.Component { } return extensions.map(ext => { - const { manifestPath: extId, isEnabled, manifest } = ext; + const { id, isEnabled, manifest } = ext; const { name, description } = manifest; + const isUninstalling = this.extensionState.get(id)?.state === "uninstalling"; return ( -
+
Name: {name} @@ -320,12 +365,12 @@ export class Extensions extends React.Component {
{!isEnabled && ( - + )} {isEnabled && ( - + )} -
diff --git a/src/renderer/components/button/button.scss b/src/renderer/components/button/button.scss index f586a03c5a..b850a3be59 100644 --- a/src/renderer/components/button/button.scss +++ b/src/renderer/components/button/button.scss @@ -12,6 +12,7 @@ flex-shrink: 0; line-height: 1; font-size: $font-size; + user-select: none; &[href] { display: inline-block; diff --git a/yarn.lock b/yarn.lock index 6e39a26a83..caaf0bd5d2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9879,10 +9879,10 @@ mobx@^5.15.4: resolved "https://registry.yarnpkg.com/mobx/-/mobx-5.15.4.tgz#9da1a84e97ba624622f4e55a0bf3300fb931c2ab" integrity sha512-xRFJxSU2Im3nrGCdjSuOTFmxVDGeqOHL+TyADCGbT0k4HHqGmx5u2yaHNryvoORpI4DfbzjJ5jPmuv+d7sioFw== -mobx@^5.15.5: - version "5.15.5" - resolved "https://registry.yarnpkg.com/mobx/-/mobx-5.15.5.tgz#69715dc8662f64d153309bfe95169b8df4b4be4b" - integrity sha512-hzk17T+/IIYLPWClRcfoA6Q5aZhFpDCr1oh8RZzu+esWP77IX/lS0V/Ee1Np+aOPKFfbSInF0reHH0L/aFfSrw== +mobx@^5.15.7: + version "5.15.7" + resolved "https://registry.yarnpkg.com/mobx/-/mobx-5.15.7.tgz#b9a5f2b6251f5d96980d13c78e9b5d8d4ce22665" + integrity sha512-wyM3FghTkhmC+hQjyPGGFdpehrcX1KOXsDuERhfK2YbJemkUhEB+6wzEN639T21onxlfYBmriA1PFnvxTUhcKw== mock-fs@^4.12.0: version "4.12.0"