fix: Ensure ErrorBoundary and PanelErrorBoundary do not throw (#2345)

- If either were asked to render `undefined` children, they would throw.
- They should be catching all errors, so we definitely do not want them
to throw. They should be more robust.
- Added unit tests.

Author: mofojed

packages/components/src/ErrorBoundary.test.tsx

@@ -40,3 +40,9 @@ it('should render the fallback if there is an error', () => {
   expect(getByText('Fallback')).toBeInTheDocument();
   expect(onError).toHaveBeenCalledWith(error, expect.anything());
 });
+
+it('should not throw if children are undefined', () => {
+  expect(() =>
+    render(<ErrorBoundary>{undefined}</ErrorBoundary>)
+  ).not.toThrow();
+});

packages/components/src/ErrorBoundary.tsx

@@ -63,7 +63,10 @@ export class ErrorBoundary extends Component<
         </div>
       );
     }
-    return children;
+
+    // We need to check for undefined children because React will throw an error if we return undefined from a render method
+    // Note this behaviour was changed in React 18: https://github.com/reactwg/react-18/discussions/75
+    return children ?? null;
   }
 }
 

packages/dashboard/src/PanelErrorBoundary.test.tsx

@@ -0,0 +1,114 @@
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+import { LoadingOverlay } from '@deephaven/components';
+import type { Container, EventEmitter } from '@deephaven/golden-layout';
+import { TestUtils } from '@deephaven/test-utils';
+import PanelErrorBoundary from './PanelErrorBoundary';
+import PanelEvent from './PanelEvent';
+import LayoutUtils from './layout/LayoutUtils';
+
+jest.mock('@deephaven/components', () => ({
+  LoadingOverlay: jest.fn(() => null),
+}));
+
+jest.mock('./layout/LayoutUtils', () => ({
+  getIdFromContainer: jest.fn(),
+}));
+
+describe('PanelErrorBoundary', () => {
+  const mockPanelId = 'test-panel-id';
+  const mockGlContainer = {
+    // Add minimal container implementation
+    on: jest.fn(),
+    off: jest.fn(),
+  } as unknown as Container;
+
+  const mockGlEventHub = {
+    emit: jest.fn(),
+  } as unknown as EventEmitter;
+
+  const TestChild = function TestChildComponent() {
+    return <div>Test Child Content</div>;
+  };
+  const ErrorChild = () => {
+    throw new Error('Test error');
+  };
+
+  beforeAll(() => {
+    TestUtils.disableConsoleOutput();
+  });
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    (LayoutUtils.getIdFromContainer as jest.Mock).mockReturnValue(mockPanelId);
+  });
+
+  afterAll(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('renders children when there is no error', () => {
+    render(
+      <PanelErrorBoundary
+        glContainer={mockGlContainer}
+        glEventHub={mockGlEventHub}
+      >
+        <TestChild />
+      </PanelErrorBoundary>
+    );
+
+    expect(screen.getByText('Test Child Content')).toBeInTheDocument();
+  });
+
+  it('renders error overlay when there is an error', () => {
+    render(
+      <PanelErrorBoundary
+        glContainer={mockGlContainer}
+        glEventHub={mockGlEventHub}
+      >
+        <ErrorChild />
+      </PanelErrorBoundary>
+    );
+
+    expect(LoadingOverlay).toHaveBeenCalledWith(
+      expect.objectContaining({
+        errorMessage: 'Error: Test error',
+        isLoading: false,
+        isLoaded: false,
+      }),
+      expect.any(Object)
+    );
+  });
+
+  it('emits CLOSED event on unmount', () => {
+    const { unmount } = render(
+      <PanelErrorBoundary
+        glContainer={mockGlContainer}
+        glEventHub={mockGlEventHub}
+      >
+        <TestChild />
+      </PanelErrorBoundary>
+    );
+
+    unmount();
+
+    expect(mockGlEventHub.emit).toHaveBeenCalledWith(
+      PanelEvent.CLOSED,
+      mockPanelId,
+      mockGlContainer
+    );
+  });
+
+  it('should not throw if children are undefined', () => {
+    expect(() =>
+      render(
+        <PanelErrorBoundary
+          glContainer={mockGlContainer}
+          glEventHub={mockGlEventHub}
+        >
+          {undefined}
+        </PanelErrorBoundary>
+      )
+    ).not.toThrow();
+  });
+});

packages/dashboard/src/PanelErrorBoundary.tsx

@@ -57,7 +57,10 @@ class PanelErrorBoundary extends Component<
         </div>
       );
     }
-    return children;
+
+    // We need to check for undefined children because React will throw an error if we return undefined from a render method
+    // Note this behaviour was changed in React 18: https://github.com/reactwg/react-18/discussions/75
+    return children ?? null;
   }
 }