Error boundaries: show Attempt Recovery option only if onError supplied

WordPress · Jan 30, 2023 · aa9cd41 · aa9cd41
1 parent 539d35f
commit aa9cd41
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 70 deletions.
diff --git a/packages/edit-site/src/components/error-boundary/index.js b/packages/edit-site/src/components/error-boundary/index.js
@@ -14,8 +14,6 @@ export default class ErrorBoundary extends Component {
 	constructor() {
 		super( ...arguments );
 
-		this.reboot = this.reboot.bind( this );
-
 		this.state = {
 			error: null,
 		};
@@ -29,13 +27,8 @@ export default class ErrorBoundary extends Component {
 		return { error };
 	}
 
-	reboot() {
-		this.props.onError();
-	}
-
 	render() {
-		const { error } = this.state;
-		if ( ! error ) {
+		if ( ! this.state.error ) {
 			return this.props.children;
 		}
 
@@ -44,8 +37,8 @@ export default class ErrorBoundary extends Component {
 				message={ __(
 					'The editor has encountered an unexpected error.'
 				) }
-				error={ error }
-				reboot={ this.reboot }
+				error={ this.state.error }
+				reboot={ this.props.onError }
 			/>
 		);
 	}

diff --git a/packages/edit-widgets/src/components/error-boundary/index.js b/packages/edit-widgets/src/components/error-boundary/index.js
@@ -17,51 +17,62 @@ function CopyButton( { text, children } ) {
 	);
 }
 
+function ErrorBoundaryWarning( { message, error, reboot } ) {
+	const actions = [];
+
+	if ( reboot ) {
+		actions.push(
+			<Button key="recovery" onClick={ reboot } variant="secondary">
+				{ __( 'Attempt Recovery' ) }
+			</Button>
+		);
+	}
+
+	if ( error ) {
+		actions.push(
+			<CopyButton key="copy-error" text={ error.stack }>
+				{ __( 'Copy Error' ) }
+			</CopyButton>
+		);
+	}
+
+	return (
+		<Warning className="edit-widgets-error-boundary" actions={ actions }>
+			{ message }
+		</Warning>
+	);
+}
+
 export default class ErrorBoundary extends Component {
 	constructor() {
 		super( ...arguments );
 
-		this.reboot = this.reboot.bind( this );
-
 		this.state = {
 			error: null,
 		};
 	}
 
 	componentDidCatch( error ) {
-		this.setState( { error } );
-
 		doAction( 'editor.ErrorBoundary.errorLogged', error );
 	}
 
-	reboot() {
-		this.props.onError();
+	static getDerivedStateFromError( error ) {
+		return { error };
 	}
 
 	render() {
-		const { error } = this.state;
-		if ( ! error ) {
+		if ( ! this.state.error ) {
 			return this.props.children;
 		}
 
 		return (
-			<Warning
-				className="edit-widgets-error-boundary"
-				actions={ [
-					<Button
-						key="recovery"
-						onClick={ this.reboot }
-						variant="secondary"
-					>
-						{ __( 'Attempt Recovery' ) }
-					</Button>,
-					<CopyButton key="copy-error" text={ error.stack }>
-						{ __( 'Copy Error' ) }
-					</CopyButton>,
-				] }
-			>
-				{ __( 'The editor has encountered an unexpected error.' ) }
-			</Warning>
+			<ErrorBoundaryWarning
+				message={ __(
+					'The editor has encountered an unexpected error.'
+				) }
+				error={ this.state.error }
+				reboot={ this.props.onError }
+			/>
 		);
 	}
 }
diff --git a/packages/editor/src/components/error-boundary/index.js b/packages/editor/src/components/error-boundary/index.js
@@ -14,6 +14,18 @@ import { doAction } from '@wordpress/hooks';
  */
 import { store as editorStore } from '../../store';
 
+function getContent() {
+	try {
+		// While `select` in a component is generally discouraged, it is
+		// used here because it (a) reduces the chance of data loss in the
+		// case of additional errors by performing a direct retrieval and
+		// (b) avoids the performance cost associated with unnecessary
+		// content serialization throughout the lifetime of a non-erroring
+		// application.
+		return select( editorStore ).getEditedPostContent();
+	} catch ( error ) {}
+}
+
 function CopyButton( { text, children } ) {
 	const ref = useCopyToClipboard( text );
 	return (
@@ -27,7 +39,6 @@ class ErrorBoundary extends Component {
 	constructor() {
 		super( ...arguments );
 
-		this.reboot = this.reboot.bind( this );
 		this.getContent = this.getContent.bind( this );
 
 		this.state = {
@@ -36,25 +47,11 @@ class ErrorBoundary extends Component {
 	}
 
 	componentDidCatch( error ) {
-		this.setState( { error } );
-
 		doAction( 'editor.ErrorBoundary.errorLogged', error );
 	}
 
-	reboot() {
-		this.props.onError();
-	}
-
-	getContent() {
-		try {
-			// While `select` in a component is generally discouraged, it is
-			// used here because it (a) reduces the chance of data loss in the
-			// case of additional errors by performing a direct retrieval and
-			// (b) avoids the performance cost associated with unnecessary
-			// content serialization throughout the lifetime of a non-erroring
-			// application.
-			return select( editorStore ).getEditedPostContent();
-		} catch ( error ) {}
+	static getDerivedStateFromError( error ) {
+		return { error };
 	}
 
 	render() {
@@ -63,25 +60,34 @@ class ErrorBoundary extends Component {
 			return this.props.children;
 		}
 
+		const actions = [];
+
+		if ( this.props.onError ) {
+			actions.push(
+				<Button
+					key="recovery"
+					onClick={ this.props.onError }
+					variant="secondary"
+				>
+					{ __( 'Attempt Recovery' ) }
+				</Button>
+			);
+		}
+
+		actions.push(
+			<CopyButton key="copy-post" text={ getContent }>
+				{ __( 'Copy Post Text' ) }
+			</CopyButton>
+		);
+
+		actions.push(
+			<CopyButton key="copy-error" text={ error.stack }>
+				{ __( 'Copy Error' ) }
+			</CopyButton>
+		);
+
 		return (
-			<Warning
-				className="editor-error-boundary"
-				actions={ [
-					<Button
-						key="recovery"
-						onClick={ this.reboot }
-						variant="secondary"
-					>
-						{ __( 'Attempt Recovery' ) }
-					</Button>,
-					<CopyButton key="copy-post" text={ this.getContent }>
-						{ __( 'Copy Post Text' ) }
-					</CopyButton>,
-					<CopyButton key="copy-error" text={ error.stack }>
-						{ __( 'Copy Error' ) }
-					</CopyButton>,
-				] }
-			>
+			<Warning className="editor-error-boundary" actions={ actions }>
 				{ __( 'The editor has encountered an unexpected error.' ) }
 			</Warning>
 		);