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

Add a global ErrorBoundary #9799

Merged
merged 12 commits into from
May 3, 2024
Merged

Add a global ErrorBoundary #9799

merged 12 commits into from
May 3, 2024

Conversation

erwanMarmelab
Copy link
Contributor

@erwanMarmelab erwanMarmelab commented Apr 29, 2024

Problem

Some errors raised in the AppBar or the Side Menu break the application in an ugly way. The error boundary in the default layout doesn't work in this case because it is designed to replace only the main content and keep rendering the navigation menu to let users change location.

Solution

Add a global ErrorBoundary before rendering the layout to catch errors in the Chrome.

TODO

  • Document Layout.md
  • Document Admin.md
  • Modify /ra-core/src/core/CoreAdminUI.tsx
  • Modify /ra-core/src/core/CoreAdmin.tsx
  • Modify /ra-ui-materialui/src/AdminUI.tsx
  • Modify /react-admin/src/Admin.tsx
  • Add JS Doc in CoreAdminUI.tsx
  • Add a story to show a catched bug

docs/Admin.md Outdated
| `darkTheme` | Optional | `object` | `default DarkTheme` | The dark theme configuration |
| `defaultTheme` | Optional | `boolean` | `false` | Flag to default to the light theme |
| `disableTelemetry` | Optional | `boolean` | `false` | Set to `true` to disable telemetry collection |
| `error` | Optional | `(props: FallbackProps) => Component` | - | A React component rendered in the content area in case of error |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the type is too precise and therefore takes too much space. Use Component as for the other similar props

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

docs/Admin.md Outdated Show resolved Hide resolved
docs/Admin.md Outdated Show resolved Hide resolved
docs/Admin.md Outdated Show resolved Hide resolved
docs/Admin.md Outdated Show resolved Hide resolved
packages/ra-core/src/core/CoreAdminUI.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/AdminUI.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/AdminUI.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/AdminUI.tsx Outdated Show resolved Hide resolved
packages/react-admin/src/Admin.stories.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@djhi djhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether we should also remove the existing ErrorBoundary from the Layout now that we have this new one. @fzaninotto?

@fzaninotto
Copy link
Member

I think we should keep the one on the layout, because it allows users to navigate using the menu after an error.

@fzaninotto fzaninotto merged commit f2a60da into next May 3, 2024
12 checks passed
@fzaninotto fzaninotto deleted the feat/global-errorboundary branch May 3, 2024 09:05
@fzaninotto fzaninotto added this to the 5.0.0 milestone May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants