From ca6810a0e2dcaa40960ad6b2550659865f1f1653 Mon Sep 17 00:00:00 2001
From: Anselm McClain <atm@xh.io>
Date: Sat, 23 Nov 2024 05:42:40 -0800
Subject: [PATCH] Fixes to DashContainerModel state handling + renders (#3839)

+ State updates were being dropped if new state applied during re-render, when ref not available.
+ Observed in use case with ViewManager, where the parent component was being moved in the layout at same time as new dash state applied.
+ Don't ignore state updates if not rendered - store and save for later application.
+ But improve ref reaction and async state application code to avoid unintended / overlapping application of state when both state and render changing at the same time.
---
 .../cmp/dash/container/DashContainerModel.ts  | 66 ++++++++++---------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/desktop/cmp/dash/container/DashContainerModel.ts b/desktop/cmp/dash/container/DashContainerModel.ts
index 4945d5003..ed3def675 100644
--- a/desktop/cmp/dash/container/DashContainerModel.ts
+++ b/desktop/cmp/dash/container/DashContainerModel.ts
@@ -214,8 +214,12 @@ export class DashContainerModel
         // Initialize GoldenLayout with initial state once ref is ready
         this.addReaction(
             {
-                track: () => [this.containerRef.current, this.layoutLocked],
-                run: () => this.loadStateAsync(this.state)
+                track: () => [this.containerRef.current, this.layoutLocked] as const,
+                run: ([ref, locked]) => {
+                    // This reaction is intended to run when ref becomes available.
+                    // It's a no-op if ref *removed* due to component re-render.
+                    if (ref) this.loadStateAsync(this.state);
+                }
             },
             {
                 track: () => this.viewState,
@@ -239,32 +243,36 @@ export class DashContainerModel
         await this.loadStateAsync(restoreState.initialState);
     }
 
-    /**
-     * Load state into the DashContainer, recreating its layout and contents
-     * @param state - State to load
-     */
+    /** Load state into the DashContainer, recreating its layout and contents */
     async loadStateAsync(state: DashViewState[]) {
-        const containerEl = this.containerRef.current;
-        if (!containerEl) {
-            this.logWarn(
-                'DashboardContainer not yet rendered - cannot update state - change will be discarded!'
-            );
-            return;
-        }
+        // Always save a reference to the state, even if the container is not yet rendered.
+        // Allows ref reaction on this class to loop back and apply it once GL is ready.
+        runInAction(() => (this.state = state));
 
-        // Show mask to provide user feedback
-        return wait()
-            .thenAction(() => {
-                this.destroyGoldenLayout();
-                this.goldenLayout = this.createGoldenLayout(containerEl, state);
-            })
-            .wait(500)
-            .then(() => {
+        // DOM required from this point on to recreate GL with new state.
+        const containerEl = this.containerRef.current;
+        if (!containerEl) return;
+
+        // Use of async below requires we check after each wait to ensure we haven't re-rendered
+        // again with a new ref. Bail out if so - ref reaction will re-enter and complete the job.
+        const refIsStale = () => this.containerRef.current !== containerEl;
+
+        return (
+            wait()
+                .thenAction(() => {
+                    if (refIsStale()) return;
+                    this.destroyGoldenLayout();
+                    this.goldenLayout = this.createGoldenLayout(containerEl, state);
+                })
                 // Since React v18, it's necessary to wait a short while for ViewModels to be available.
-                this.refreshActiveViews();
-                this.updateTabHeaders();
-            })
-            .linkTo(this.loadingStateTask);
+                .wait(500)
+                .then(() => {
+                    if (refIsStale()) return;
+                    this.refreshActiveViews();
+                    this.updateTabHeaders();
+                })
+                .linkTo(this.loadingStateTask)
+        );
     }
 
     /**
@@ -343,13 +351,7 @@ export class DashContainerModel
 
     setPersistableState(persistableState: PersistableState<{state: DashViewState[]}>) {
         const {state} = persistableState.value;
-        if (!state) return;
-        if (this.containerRef.current) {
-            this.loadStateAsync(state);
-        } else {
-            // If the container is not yet rendered, store the state directly
-            this.state = state;
-        }
+        if (state) this.loadStateAsync(state);
     }
 
     //------------------------