fix: Dehydration of class components (#1535)

Fixes #1534

Author: mattrunyon

packages/dashboard/src/DashboardLayout.tsx

@@ -28,6 +28,7 @@ import PanelEvent from './PanelEvent';
 import { GLPropTypes, useListener } from './layout';
 import { getDashboardData, updateDashboardData } from './redux';
 import {
+  isWrappedComponent,
   PanelComponentType,
   PanelDehydrateFunction,
   PanelHydrateFunction,
@@ -118,24 +119,49 @@ export function DashboardLayout({
         componentDehydrate
       );
 
-      function wrappedComponent(props: PanelProps): JSX.Element {
-        const CType = componentType;
+      function wrappedComponent(
+        props: PanelProps,
+        ref: React.Ref<unknown>
+      ): JSX.Element {
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        const CType = componentType as any;
         const PanelWrapperType = panelWrapper;
 
+        /*
+          Checking for class components will let us silence the React warning
+          about assigning refs to function components not using forwardRef.
+          The ref is used to detect changes to class component state so we
+          can track changes to panelState. We should opt for more explicit
+          state changes in the future and in functional components.
+        */
+        const isClassComponent =
+          (isWrappedComponent(CType) &&
+            CType.WrappedComponent.prototype != null &&
+            CType.WrappedComponent.prototype.isReactComponent != null) ||
+          (CType.prototype != null && CType.prototype.isReactComponent != null);
+
         // Props supplied by GoldenLayout
         const { glContainer, glEventHub } = props;
         return (
           <PanelErrorBoundary glContainer={glContainer} glEventHub={glEventHub}>
             {/* eslint-disable-next-line react/jsx-props-no-spreading */}
             <PanelWrapperType {...props}>
-              {/* eslint-disable-next-line react/jsx-props-no-spreading */}
-              <CType {...props} />
+              {isClassComponent ? (
+                // eslint-disable-next-line react/jsx-props-no-spreading
+                <CType {...props} ref={ref} />
+              ) : (
+                // eslint-disable-next-line react/jsx-props-no-spreading
+                <CType {...props} />
+              )}
             </PanelWrapperType>
           </PanelErrorBoundary>
         );
       }
 
-      const cleanup = layout.registerComponent(name, wrappedComponent);
+      const cleanup = layout.registerComponent(
+        name,
+        React.forwardRef(wrappedComponent)
+      );
       hydrateMap.set(name, componentHydrate);
       dehydrateMap.set(name, componentDehydrate);
       return cleanup;

packages/golden-layout/src/utils/ReactComponentHandler.tsx

@@ -164,6 +164,17 @@ export default class ReactComponentHandler {
     var defaultProps = {
       glEventHub: this._container.layoutManager.eventHub,
       glContainer: this._container,
+      /**
+       * This ref assignment makes use of callback refs which is a legacy ref style in React.
+       * It uses the callback to inject GoldenLayout _onUpdate into the React componentWillUpdate lifecycle.
+       * This allows GoldenLayout to track the internal state of class components.
+       * We then emit this state change and somewhere furter up, serialize it.
+       * Specifically we look for state.panelState changes.
+       * We should not do this going forward as it's quite unclear where/why your component's state might be saved.
+       * This ref cannot be removed unless we refactor all existing panels to not rely on the magic of panelState.
+       * DashboardUtils#dehydrate is where the panelState gets read/saved.
+       */
+      ref: this._gotReactComponent.bind(this),
     };
     var props = $.extend(defaultProps, this._container._config.props);
     return React.createElement(this._reactClass, props);

tests/notebook.spec.ts

@@ -2,7 +2,7 @@ import { test, expect } from '@playwright/test';
 import shortid from 'shortid';
 import { pasteInMonaco } from './utils';
 
-test('test creating a file, saving it, closing it, re-opening it, running it, then deleting it', async ({
+test('test creating a file, saving it, reloading the page, closing it, re-opening it, running it, then deleting it', async ({
   page,
 }) => {
   await page.goto('');
@@ -33,6 +33,19 @@ test('test creating a file, saving it, closing it, re-opening it, running it, th
   // Click button:has-text("Save")
   await page.locator('button:has-text("Save")').click();
 
+  // Pause so the save can actually finish
+  // We eagerly show the save status, so can't use that to detect save finish
+  await new Promise(resolve => {
+    setTimeout(resolve, 2000);
+  });
+
+  // Reload page to check state of panel is saved
+  await page.reload();
+
+  await expect(
+    page.locator('.editor-container').locator('textarea')
+  ).not.toBeEmpty();
+
   // Click close on the notebook file .lm_close_tab
   await page.locator('.lm_close_tab').click();