Address PR review comments on error boundaries
Guard ErrorBoundary against a falsy thrown value: track a separate
hasError flag and accept the caught value as unknown, so throwing null,
0, or "" still renders the fallback instead of looping back into the
failing subtree.
Add role="alert" to InlineError and ErrorState so assistive tech
announces errors that appear dynamically after a fetch or query failure.
Fix the RecentUpdatesSection doc example, which showed onRetry={reset}
with @throwOnFieldError even though reset cannot clear a field error;
use a reload and point to the retry table.
Signed-off-by: Émile Ré <emile@probo.com>
This commit is contained in:
@@ -33,34 +33,37 @@ import { Component, type ErrorInfo, type ReactNode } from "react";
|
|||||||
|
|
||||||
export interface ErrorBoundaryProps {
|
export interface ErrorBoundaryProps {
|
||||||
children: ReactNode;
|
children: ReactNode;
|
||||||
// A node, or a render function that receives the caught error + a reset fn.
|
// A node, or a render function that receives the caught value + a reset fn.
|
||||||
fallback?: ReactNode | ((error: Error, reset: () => void) => ReactNode);
|
// `unknown` because anything can be thrown, not just an Error.
|
||||||
onError?: (error: Error, info: ErrorInfo) => void;
|
fallback?: ReactNode | ((error: unknown, reset: () => void) => ReactNode);
|
||||||
|
onError?: (error: unknown, info: ErrorInfo) => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
interface ErrorBoundaryState {
|
interface ErrorBoundaryState {
|
||||||
error: Error | null;
|
// Tracked separately from `error` so a falsy thrown value (null, 0, "") still
|
||||||
|
// renders the fallback instead of looping back into the failing subtree.
|
||||||
|
hasError: boolean;
|
||||||
|
error: unknown;
|
||||||
}
|
}
|
||||||
|
|
||||||
export class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState> {
|
export class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState> {
|
||||||
state: ErrorBoundaryState = { error: null };
|
state: ErrorBoundaryState = { hasError: false, error: null };
|
||||||
|
|
||||||
static getDerivedStateFromError(error: Error): ErrorBoundaryState {
|
static getDerivedStateFromError(error: unknown): ErrorBoundaryState {
|
||||||
return { error };
|
return { hasError: true, error };
|
||||||
}
|
}
|
||||||
|
|
||||||
componentDidCatch(error: Error, info: ErrorInfo) {
|
componentDidCatch(error: unknown, info: ErrorInfo) {
|
||||||
this.props.onError?.(error, info);
|
this.props.onError?.(error, info);
|
||||||
}
|
}
|
||||||
|
|
||||||
reset = () => this.setState({ error: null });
|
reset = () => this.setState({ hasError: false, error: null });
|
||||||
|
|
||||||
render() {
|
render() {
|
||||||
const { error } = this.state;
|
if (this.state.hasError) {
|
||||||
if (error) {
|
|
||||||
const { fallback } = this.props;
|
const { fallback } = this.props;
|
||||||
if (typeof fallback === "function") {
|
if (typeof fallback === "function") {
|
||||||
return fallback(error, this.reset);
|
return fallback(this.state.error, this.reset);
|
||||||
}
|
}
|
||||||
return fallback ?? null;
|
return fallback ?? null;
|
||||||
}
|
}
|
||||||
@@ -188,8 +191,15 @@ export function RecentUpdatesSection({ trustCenterKey }: Props) {
|
|||||||
const { t } = useTranslation();
|
const { t } = useTranslation();
|
||||||
return (
|
return (
|
||||||
<ErrorBoundary
|
<ErrorBoundary
|
||||||
fallback={(_, reset) => (
|
// This section reads a field of the preloaded page query — there is no
|
||||||
<InlineError message={t("errors.inline.message")} onRetry={reset} retryLabel={t("errors.inline.retry")} />
|
// local refetch, so recover with a reload, not the boundary's `reset`
|
||||||
|
// (see "Retrying: reset vs refetch vs reload" below).
|
||||||
|
fallback={(
|
||||||
|
<InlineError
|
||||||
|
message={t("errors.inline.message")}
|
||||||
|
retryLabel={t("errors.inline.retry")}
|
||||||
|
onRetry={() => window.location.reload()}
|
||||||
|
/>
|
||||||
)}
|
)}
|
||||||
>
|
>
|
||||||
<RecentUpdatesSectionContent trustCenterKey={trustCenterKey} />
|
<RecentUpdatesSectionContent trustCenterKey={trustCenterKey} />
|
||||||
|
|||||||
@@ -16,37 +16,40 @@ import { Component, type ErrorInfo, type ReactNode } from "react";
|
|||||||
|
|
||||||
export interface ErrorBoundaryProps {
|
export interface ErrorBoundaryProps {
|
||||||
children: ReactNode;
|
children: ReactNode;
|
||||||
// A node, or a render function that receives the caught error + a reset fn.
|
// A node, or a render function that receives the caught value + a reset fn.
|
||||||
fallback?: ReactNode | ((error: Error, reset: () => void) => ReactNode);
|
// The value is `unknown` because anything can be thrown, not just an Error.
|
||||||
onError?: (error: Error, info: ErrorInfo) => void;
|
fallback?: ReactNode | ((error: unknown, reset: () => void) => ReactNode);
|
||||||
|
onError?: (error: unknown, info: ErrorInfo) => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
interface ErrorBoundaryState {
|
interface ErrorBoundaryState {
|
||||||
error: Error | null;
|
// Tracked separately from `error` so a falsy thrown value (null, 0, "") still
|
||||||
|
// renders the fallback instead of looping back into the failing subtree.
|
||||||
|
hasError: boolean;
|
||||||
|
error: unknown;
|
||||||
}
|
}
|
||||||
|
|
||||||
// The single reusable error boundary primitive (the sanctioned use of a class).
|
// The single reusable error boundary primitive (the sanctioned use of a class).
|
||||||
// Generic — works at bootstrap, route, section, or component level; only the
|
// Generic — works at bootstrap, route, section, or component level; only the
|
||||||
// placement and the `fallback` differ. See contrib/claude/error-handling.md.
|
// placement and the `fallback` differ. See contrib/claude/error-handling.md.
|
||||||
export class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState> {
|
export class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState> {
|
||||||
state: ErrorBoundaryState = { error: null };
|
state: ErrorBoundaryState = { hasError: false, error: null };
|
||||||
|
|
||||||
static getDerivedStateFromError(error: Error): ErrorBoundaryState {
|
static getDerivedStateFromError(error: unknown): ErrorBoundaryState {
|
||||||
return { error };
|
return { hasError: true, error };
|
||||||
}
|
}
|
||||||
|
|
||||||
componentDidCatch(error: Error, info: ErrorInfo) {
|
componentDidCatch(error: unknown, info: ErrorInfo) {
|
||||||
this.props.onError?.(error, info);
|
this.props.onError?.(error, info);
|
||||||
}
|
}
|
||||||
|
|
||||||
reset = () => this.setState({ error: null });
|
reset = () => this.setState({ hasError: false, error: null });
|
||||||
|
|
||||||
render() {
|
render() {
|
||||||
const { error } = this.state;
|
if (this.state.hasError) {
|
||||||
if (error) {
|
|
||||||
const { fallback } = this.props;
|
const { fallback } = this.props;
|
||||||
if (typeof fallback === "function") {
|
if (typeof fallback === "function") {
|
||||||
return fallback(error, this.reset);
|
return fallback(this.state.error, this.reset);
|
||||||
}
|
}
|
||||||
return fallback ?? null;
|
return fallback ?? null;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -39,7 +39,7 @@ export function ErrorState({ code, title, description, actions, fullPage, classN
|
|||||||
const slots = errorState({ fullPage });
|
const slots = errorState({ fullPage });
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className={slots.root({ className })}>
|
<div role="alert" className={slots.root({ className })}>
|
||||||
<div className={slots.block()}>
|
<div className={slots.block()}>
|
||||||
<div className={slots.content()}>
|
<div className={slots.content()}>
|
||||||
{code && (
|
{code && (
|
||||||
|
|||||||
@@ -35,7 +35,7 @@ export function InlineError({ layout, message, onRetry, retryLabel = "Retry", cl
|
|||||||
const slots = inlineError({ layout });
|
const slots = inlineError({ layout });
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className={slots.root({ className })}>
|
<div role="alert" className={slots.root({ className })}>
|
||||||
<Text size={2} color="neutral" className={slots.message()}>{message}</Text>
|
<Text size={2} color="neutral" className={slots.message()}>{message}</Text>
|
||||||
{onRetry && (
|
{onRetry && (
|
||||||
<Button size={2} variant="soft" color="neutral" onClick={onRetry}>
|
<Button size={2} variant="soft" color="neutral" onClick={onRetry}>
|
||||||
|
|||||||
Reference in New Issue
Block a user