refactor: error boundary

src/plugins/Plugin.jsx

@@ -4,8 +4,7 @@ import React, {
   useEffect, useMemo, useState,
 } from 'react';
 import PropTypes from 'prop-types';
-import { ErrorBoundary } from 'react-error-boundary';
-import { logError } from '@edx/frontend-platform/logging';
+import { ErrorBoundary } from '@edx/frontend-platform/react';
 import {
   injectIntl,
   intlShape,
@@ -17,19 +16,23 @@ import {
 import { PLUGIN_RESIZE } from './data/constants';
 import messages from './Plugins.messages';
 
-// TODO: see example-plugin-app/src/PluginOne.jsx for example of customizing errorFallback
-function errorFallbackDefault(intl) {
-  return (
-    <div>
-      <h2>
-        {intl.formatMessage(messages.unexpectedError)}
-      </h2>
-    </div>
-  );
-}
-
+// TODO: create example-plugin-app/src/PluginOne.jsx for example of customizing errorFallback
+const ErrorFallbackDefault = ({ intl }) => (
+  <div>
+    <h2>
+      {intl.formatMessage(messages.unexpectedError)}
+    </h2>
+  </div>
+);
+// NOTES:
+// I changed the errorFallBackProp to ErrorFallbackComponent
+// It is now capitalized to signify that it is a react component
+// The name is changed for the same reason
+// The reason why I wanted to make this clear is because the <ErrorBoundary /> component
+// from frontend-platform wouldn't render anything unless it was provided
+// as a React Component (ie. <ErrorFallback />) see this below in the final render of <Plugin />
 const Plugin = ({
-  children, className, intl, style, ready, errorFallbackProp,
+  children, className, intl, style, ready, ErrorFallbackComponent,
 }) => {
   const [dimensions, setDimensions] = useState({
     width: null,
@@ -41,13 +44,10 @@ const Plugin = ({
     ...style,
   }), [dimensions, style]);
 
-  const errorFallback = errorFallbackProp || errorFallbackDefault;
-
-  // Error logging function
   // Need to confirm: When an error is caught here, the logging will be sent to the child MFE's logging service
-  const logErrorToService = (error, info) => {
-    logError(error, { stack: info.componentStack });
-  };
+
+  // NOTES: capitalized here for same reason — it's not necessary but helps devs remember
+  const ErrorFallback = ErrorFallbackComponent || ErrorFallbackDefault;
 
   useHostEvent(PLUGIN_RESIZE, ({ payload }) => {
     setDimensions({
@@ -73,8 +73,12 @@ const Plugin = ({
   return (
     <div className={className} style={finalStyle}>
       <ErrorBoundary
-        FallbackComponent={() => errorFallback(intl)}
-        onError={logErrorToService}
+      // NOTES:
+      // If the prop provided here is not in React Component format (<ComponentName ...props />)
+      // then it won't be rendered to the page
+      // TODO: update frontend-platform code to refactor this
+      // or include info in docs somewhere
+        fallbackComponent={<ErrorFallback intl={intl} />}
       >
         {children}
       </ErrorBoundary>
@@ -87,15 +91,19 @@ export default injectIntl(Plugin);
 Plugin.propTypes = {
   children: PropTypes.node.isRequired,
   className: PropTypes.string,
-  errorFallbackProp: PropTypes.func,
+  ErrorFallbackComponent: PropTypes.func,
   intl: intlShape.isRequired,
   ready: PropTypes.bool,
   style: PropTypes.object, // eslint-disable-line
 };
 
 Plugin.defaultProps = {
   className: null,
-  errorFallbackProp: null,
+  ErrorFallbackComponent: null,
   style: {},
   ready: true,
 };
+
+ErrorFallbackDefault.propTypes = {
+  intl: intlShape.isRequired,
+};

src/plugins/PluginContainerIframe.jsx

@@ -71,7 +71,7 @@ const PluginContainerIframe = ({
         scrolling={scrolling}
         referrerPolicy="origin" // The sent referrer will be limited to the origin of the referring page: its scheme, host, and port.
         className={classNames(
-          'border border-0',
+          'border border-0 w-100',
           { 'd-none': !ready },
           className,
         )}

src/plugins/PluginSlot.jsx

@@ -21,17 +21,25 @@ const PluginSlot = forwardRef(({
   */
   const { plugins, keepDefault } = usePluginSlot(id);
 
-  const { fallback } = pluginProps;
+  // NOTES:
+  // This is now "loadingFallback" to better show what it is used for
+  // This fallback will be rendered the page while the PLUGIN_READY value is false
+  // PLUGIN_READY event is fired when the Plugin component mounts
+  // If no components wrapped in Plugin are provided in the iframed component, this event will never fire
+  // And thus the spinner will remain on the screen with no indication to the viewer
+  // This is important to note when creating Plugins — perhaps a PluginWrapper component should be encouraged?
+  // Similar to what is used in the test files
+  // This is also what happens with errors handled by ErrorBoundary —
+  // If the code that errors is not wrapped in a Plugin component,
+  // then the error will bubble up to the ErrorBoundary in AppProvider (every MFE is wrapped in <AppProvider />)
+  const { loadingFallback } = pluginProps;
 
-  // TODO: Add internationalization to the "Loading" text on the spinner.
-  let finalFallback = (
+  const defaultLoadingFallback = (
     <div className={classNames(pluginProps.className, 'd-flex justify-content-center align-items-center')}>
       <Spinner animation="border" screenReaderText={intl.formatMessage(messages.loading)} />
     </div>
   );
-  if (fallback !== undefined) {
-    finalFallback = fallback;
-  }
+  const finalLoadingFallback = loadingFallback !== undefined ? loadingFallback : defaultLoadingFallback;
 
   let finalChildren = [];
   if (plugins.length > 0) {
@@ -43,7 +51,7 @@ const PluginSlot = forwardRef(({
         <PluginContainer
           key={pluginConfig.url}
           config={pluginConfig}
-          fallback={finalFallback}
+          fallback={finalLoadingFallback}
           {...pluginProps}
         />,
       );