Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ensure ErrorBoundary and PanelErrorBoundary do not throw #2345

Merged
merged 1 commit into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/components/src/ErrorBoundary.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
5 changes: 4 additions & 1 deletion packages/components/src/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
114 changes: 114 additions & 0 deletions packages/dashboard/src/PanelErrorBoundary.test.tsx
Original file line number Diff line number Diff line change
@@ -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();
});
});
5 changes: 4 additions & 1 deletion packages/dashboard/src/PanelErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
Loading