Remove broken symlink from node_modules on uninstall (#1695)

Signed-off-by: Panu Horsmalahti <[email protected]>

src/extensions/extension-discovery.ts

@@ -206,7 +206,7 @@ export class ExtensionDiscovery {
 
         // The path to the manifest file is the lens extension id
         // Note that we need to use the symlinked path
-        const lensExtensionId = path.join(this.nodeModulesPath, extensionName, "package.json");
+        const lensExtensionId = path.join(this.nodeModulesPath, extensionName, manifestFilename);
 
         logger.info(`${logModule} removed extension ${extensionName}`);
         this.events.emit("remove", lensExtensionId as LensExtensionId);
@@ -217,12 +217,17 @@ export class ExtensionDiscovery {
   };
 
   /**
-   * Uninstalls extension by path.
+   * Uninstalls extension.
    * The application will detect the folder unlink and remove the extension from the UI automatically.
-   * @param absolutePath Path to the non-symlinked folder of the extension
+   * @param extension Extension to unistall.
    */
-  async uninstallExtension(absolutePath: string) {
-    logger.info(`${logModule} Uninstalling ${absolutePath}`);
+  async uninstallExtension({ absolutePath, manifest }: InstalledExtension) {
+    logger.info(`${logModule} Uninstalling ${manifest.name}`);
+
+    // remove the symlink under node_modules.
+    // If we don't remove the symlink, the uninstall would leave a non-working symlink,
+    // which wouldn't be fixed if the extension was reinstalled, causing the extension not to work.
+    await fs.remove(this.getInstalledPath(manifest.name));
 
     const exists = await fs.pathExists(absolutePath);
 
@@ -269,6 +274,22 @@ export class ExtensionDiscovery {
     return extensions;
   }
 
+  /**
+   * Returns the symlinked path to the extension folder,
+   * e.g. "/Users/<username>/Library/Application Support/Lens/node_modules/@publisher/extension"
+   */
+  protected getInstalledPath(name: string) {
+    return path.join(this.nodeModulesPath, name);
+  }
+
+  /**
+   * Returns the symlinked path to the package.json,
+   * e.g. "/Users/<username>/Library/Application Support/Lens/node_modules/@publisher/extension/package.json"
+   */
+  protected getInstalledManifestPath(name: string) {
+    return path.join(this.getInstalledPath(name), manifestFilename);
+  }
+
   protected async getByManifest(manifestPath: string, { isBundled = false }: {
     isBundled?: boolean;
   } = {}): Promise<InstalledExtension | null> {
@@ -279,7 +300,7 @@ export class ExtensionDiscovery {
       fs.accessSync(manifestPath, fs.constants.F_OK);
 
       manifestJson = __non_webpack_require__(manifestPath);
-      const installedManifestPath = path.join(this.nodeModulesPath, manifestJson.name, "package.json");
+      const installedManifestPath = this.getInstalledManifestPath(manifestJson.name);
 
       this.packagesJson.dependencies[manifestJson.name] = path.dirname(manifestPath);
       const isEnabled = isBundled || extensionsStore.isEnabled(installedManifestPath);

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

@@ -68,7 +68,7 @@ describe("Extensions", () => {
     // Approve confirm dialog
     fireEvent.click(screen.getByText("Yes"));
 
-    expect(extensionDiscovery.uninstallExtension).toHaveBeenCalledWith("/absolute/path");
+    expect(extensionDiscovery.uninstallExtension).toHaveBeenCalled();
     expect(screen.getByText("Disable").closest("button")).toBeDisabled();
     expect(screen.getByText("Uninstall").closest("button")).toBeDisabled();
   });

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

@@ -413,7 +413,7 @@ export class Extensions extends React.Component {
         displayName
       });
 
-      await extensionDiscovery.uninstallExtension(extension.absolutePath);
+      await extensionDiscovery.uninstallExtension(extension);
     } catch (error) {
       Notifications.error(
         <p>Uninstalling extension <b>{displayName}</b> has failed: <em>{error?.message ?? ""}</em></p>