From c5f213df7b0d207667692395738c92c01f7e0837 Mon Sep 17 00:00:00 2001
From: Kristóf Marussy <kristof@marussy.com>
Date: Tue, 25 Jan 2022 17:56:28 +0100
Subject: refactor: Apply shared store patches in batches
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Makes sure that the renderer always sees a consistent state.

Signed-off-by: Kristóf Marussy <kristof@marussy.com>
---
 packages/main/src/index.ts              | 18 ++++++++--
 packages/main/src/stores/SharedStore.ts | 63 +++++++++------------------------
 2 files changed, 33 insertions(+), 48 deletions(-)

(limited to 'packages/main')

diff --git a/packages/main/src/index.ts b/packages/main/src/index.ts
index 0978cd9..bcdc3d7 100644
--- a/packages/main/src/index.ts
+++ b/packages/main/src/index.ts
@@ -36,7 +36,7 @@ import {
 import { app, BrowserView, BrowserWindow, ipcMain } from 'electron';
 import { ensureDirSync, readFile, readFileSync } from 'fs-extra';
 import { autorun } from 'mobx';
-import { getSnapshot, onPatch } from 'mobx-state-tree';
+import { getSnapshot, onAction, onPatch } from 'mobx-state-tree';
 import osName from 'os-name';
 
 import {
@@ -292,9 +292,23 @@ async function createWindow(): Promise<unknown> {
     }
   });
 
+  const batchedPatches: unknown[] = [];
   onPatch(store.shared, (patch) => {
-    webContents.send(MainToRendererIpcMessage.SharedStorePatch, patch);
+    batchedPatches.push(patch);
   });
+  onAction(
+    store,
+    () => {
+      if (batchedPatches.length > 0) {
+        webContents.send(
+          MainToRendererIpcMessage.SharedStorePatch,
+          batchedPatches,
+        );
+        batchedPatches.splice(0);
+      }
+    },
+    true,
+  );
 
   ipcMain.handle(ServiceToMainIpcMessage.ApiExposedInMainWorld, (event) => {
     if (event.sender.id !== browserView.webContents.id) {
diff --git a/packages/main/src/stores/SharedStore.ts b/packages/main/src/stores/SharedStore.ts
index 9ec7963..499d1ee 100644
--- a/packages/main/src/stores/SharedStore.ts
+++ b/packages/main/src/stores/SharedStore.ts
@@ -55,25 +55,20 @@ function getConfigs<T>(models: { config: T }[]): T[] | undefined {
   return models.length === 0 ? undefined : models.map((model) => model.config);
 }
 
-type TypeWithSettings<C> = IType<
-  { id: string; settings: C },
-  unknown,
-  { settings: IStateTreeNode<IType<C, unknown, unknown>> }
->;
-
-function deleteStaleModels<C, D extends TypeWithSettings<C>>(
+function applySettings<
+  C,
+  D extends IType<
+    { id: string; settings: C },
+    unknown,
+    { settings: IStateTreeNode<IType<C, unknown, unknown>> }
+  >,
+>(
+  current: IMSTArray<IReferenceType<D>>,
   currentById: IMSTMap<D>,
-  toApplyById: Map<string, C>,
+  toApply: [string, C][],
 ): void {
+  const toApplyById = new Map(toApply);
   const toDelete = new Set(currentById.keys());
-  toApplyById.forEach((_settings, id) => toDelete.delete(id));
-  toDelete.forEach((id) => currentById.delete(id));
-}
-
-function applySettings<C, D extends TypeWithSettings<C>>(
-  currentById: IMSTMap<D>,
-  toApplyById: Map<string, C>,
-): void {
   toApplyById.forEach((settingsSnapshot, id) => {
     const model = currentById.get(id);
     if (model === undefined) {
@@ -82,16 +77,13 @@ function applySettings<C, D extends TypeWithSettings<C>>(
         settings: settingsSnapshot,
       });
     } else {
+      toDelete.delete(id);
       applySnapshot(model.settings, settingsSnapshot);
     }
   });
-}
-
-function pushReferences<C, D extends TypeWithSettings<C>>(
-  list: IMSTArray<IReferenceType<D>>,
-  toApply: [string, C][],
-): void {
-  list.push(...toApply.map(([id]) => id));
+  current.clear();
+  toDelete.forEach((id) => currentById.delete(id));
+  current.push(...toApply.map(([id]) => id));
 }
 
 export const sharedStore = overrideProps(originalSharedStore, {
@@ -115,9 +107,6 @@ export const sharedStore = overrideProps(originalSharedStore, {
   }))
   .actions((self) => ({
     loadConfig(config: Config): void {
-      // `onPatch` will send store changes piecemeal without any attention to
-      // transaction boundaries. We must make sure that any state communicated to the
-      // renderer process is actually valid.
       const {
         profiles,
         profilesById,
@@ -137,27 +126,9 @@ export const sharedStore = overrideProps(originalSharedStore, {
         servicesConfig,
         profilesToApply,
       );
-      const profilesToApplyById = new Map(profilesToApply);
-      const servicesToApplyById = new Map(servicesToApply);
-      // First remove any references to profiles and services that might be deleted.
-      self.selectedService = undefined;
-      services.clear();
-      profiles.clear();
-      // Delete all services that may depend on profiles that will be delete.
-      deleteStaleModels(servicesById, servicesToApplyById);
-      // Update existing profiles and add new profiles.
-      applySettings(profilesById, profilesToApplyById);
-      // Update existing services and add new services. This will make sure that no service
-      // depends on a profile that will be deleted.
-      applySettings(servicesById, servicesToApplyById);
-      // Now it's safe to delete stale profiles.
-      deleteStaleModels(profilesById, profilesToApplyById);
-      // We are ready to build new profile and service lists from the new models.
-      pushReferences(profiles, profilesToApply);
-      pushReferences(services, servicesToApply);
-      // Global settings may refer to particular profiles or services.
+      applySettings(profiles, profilesById, profilesToApply);
+      applySettings(services, servicesById, servicesToApply);
       applySnapshot(settings, settingsToApply);
-      // Restore service selection (if applicable).
       let newSelectedService;
       if (selectedServiceId !== undefined) {
         newSelectedService = servicesById.get(selectedServiceId);
-- 
cgit v1.2.3-70-g09d2