refactor: use the broadcast channel to update the currently active ve…

…rsions
electron · Jul 22, 2023 · 96a196b · 96a196b
1 parent 4d2f516
commit 96a196b
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 48 deletions.
diff --git a/src/interfaces.ts b/src/interfaces.ts
@@ -229,9 +229,14 @@ export type AppStateBroadcastMessage =
   | {
       type: AppStateBroadcastMessageType.syncVersions;
       payload: RunnableVersion[];
+    }
+  | {
+      type: AppStateBroadcastMessageType.activeVersionsChanged;
+      payload?: never;
     };
 
 export enum AppStateBroadcastMessageType {
+  activeVersionsChanged = 'activeVersionsChanged',
   isDownloadingAll = 'isDownloadingAll',
   syncVersions = 'syncVersions',
 }
diff --git a/src/renderer/components/settings-electron.tsx b/src/renderer/components/settings-electron.tsx
@@ -52,39 +52,6 @@ export const ElectronSettings = observer(
       this.handleStateChange = this.handleStateChange.bind(this);
     }
 
-    /**
-     * Queries the currently active versions and update the local state.
-     *
-     * This currently gives a warning/error in development mode when the ElectronSettings component is unmounted
-     * ("Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak
-     * in your application"). This is a false positive, the warning has been removed in React 18+ (see
-     * https://github.com/facebook/react/pull/22114).
-     *
-     * @TODO upgrade to React 18
-     */
-    updateActiveVersions = () => {
-      this.props.appState
-        .getActiveVersions()
-        .then((activeVersions) =>
-          this.setState({ ...this.state, activeVersions }),
-        )
-        .catch((err) => {
-          console.error(
-            'Error updating the currently active Electron versions:',
-          );
-          console.error(err);
-        });
-    };
-
-    public componentDidMount() {
-      this.updateActiveVersions();
-    }
-
-    // Fired when other windows change their active Electron version
-    public componentDidUpdate() {
-      this.updateActiveVersions();
-    }
-
     public handleUpdateElectronVersions() {
       this.props.appState.updateElectronVersions();
     }
@@ -435,7 +402,7 @@ export const ElectronSettings = observer(
           break;
       }
 
-      if (this.state.activeVersions.has(version)) {
+      if (appState.activeVersions.has(version)) {
         return (
           <Tooltip
             position="auto"

diff --git a/src/renderer/state.ts b/src/renderer/state.ts
@@ -234,24 +234,25 @@ export class AppState {
 
   private static versionLockNamePrefix = 'version:';
 
+  public activeVersions: Set<string> = new Set();
+
   private getVersionLockName(ver: string) {
     return `${AppState.versionLockNamePrefix}${ver}`;
   }
 
   /**
-   * Retrieves all Electron versions that are currently active in some window.
+   * Updates the Electron versions that are currently active in some window.
    */
-  public async getActiveVersions(): Promise<Set<string>> {
-    return ((await navigator.locks.query()).held || []).reduce<Set<string>>(
-      (acc, item) => {
-        if (item.name?.startsWith(AppState.versionLockNamePrefix)) {
-          acc.add(item.name.split(AppState.versionLockNamePrefix)[1]);
-        }
+  private async updateActiveVersions(): Promise<void> {
+    this.activeVersions = ((await navigator.locks.query()).held || []).reduce<
+      Set<string>
+    >((acc, item) => {
+      if (item.name?.startsWith(AppState.versionLockNamePrefix)) {
+        acc.add(item.name.split(AppState.versionLockNamePrefix)[1]);
+      }
 
-        return acc;
-      },
-      new Set(),
-    );
+      return acc;
+    }, new Set());
   }
 
   constructor(versions: RunnableVersion[]) {
@@ -262,6 +263,7 @@ export class AppState {
       addAcceleratorToBlock: action,
       addLocalVersion: action,
       addNewVersions: action,
+      activeVersions: observable,
       channelsToShow: observable,
       clearConsole: action,
       currentElectronVersion: computed,
@@ -505,6 +507,12 @@ export class AppState {
         const { type, payload } = event.data;
 
         switch (type) {
+          case AppStateBroadcastMessageType.activeVersionsChanged: {
+            this.updateActiveVersions();
+
+            break;
+          }
+
           case AppStateBroadcastMessageType.isDownloadingAll: {
             this.isDownloadingAll = payload;
             break;
@@ -824,9 +832,7 @@ export class AppState {
   public async removeVersion(ver: RunnableVersion): Promise<void> {
     const { version, state, source } = ver;
 
-    const activeVersions = await this.getActiveVersions();
-
-    if (activeVersions.has(ver.version)) {
+    if (this.activeVersions.has(ver.version)) {
       console.log(`State: Not removing active version ${version}`);
       return;
     }
@@ -1007,6 +1013,15 @@ export class AppState {
       this.getVersionLockName(version),
       { mode: 'shared' },
       (lock) => {
+        // let other windows know we're using this version
+        this.broadcastChannel.postMessage({
+          type: AppStateBroadcastMessageType.activeVersionsChanged,
+        });
+
+        // the current window's state also needs an update - that's how
+        // the current window knows it can't remove this version
+        this.updateActiveVersions();
+
         this.hasActiveLock = Boolean(lock);
 
         /**