single-spa / single-spa-react

Single-spa lifecycles helper for React applications
https://single-spa.js.org/docs/ecosystem-react.html
MIT License
227 stars 63 forks source link

Is it possible to pass `errorBoundary` prop for single-spa-react at the point of calling singleSpa.registerApplication? #119

Closed ahayes91 closed 3 years ago

ahayes91 commented 3 years ago

Describe the bug or question We use singleSpa to register a number of singleSpaReact microfrontend apps. We would like to have a shared ErrorBoundary implementation across these apps (for consistent UX when apps fail for any reason). Ideally we would define that errorBoundary function once, and be able to supply it once at the point of registering each app.

However, it seems that customProps on singleSpa.registerApplication isn't powerful enough to supply errorBoundary to the react apps:

export const { bootstrap, mount, unmount } = singleSpaReact({
  React,
  ReactDOM,
  rootComponent: App,
  // For example, `domElementGetter` can be handled by the container that registers each app
  errorBoundary, // but errorBoundary HAS to be supplied here
});

To Reproduce For bugs, create a small git repo or codepen that reproduces the problem, and provide steps on how to see the issue. This is also helpful for questions.

I'll try and create one of these over the next couple of days to better showcase the question.

Expected behavior I'd expect to be allowed to supply a common singleSpaReact prop in the customProps attribute of registerApplication:

function errorBoundary (err) {
  return <ErrorBoundaryMessage errorMessage={err.toString()} />; // UI component for consistency in our application
}

// loop through each app and register it:
    singleSpa.registerApplication({
      name: appName,
      customProps: {
        domElementGetter: getDomElementGetter(domElement),
        errorBoundary,
      },
    });

Screenshots and/or console output It doesn't seem like the errorBoundary is supplied - if I force an error in one of my apps, it crashes.

Additional context Add any other context about the problem or question here.

joeldenning commented 3 years ago

The way I've usually done this is by putting the error boundary component in one microfrontend and then doing a cross microfrontend import from all others to get it. Alternatively, I've also published the error boundary to npm and then just rebundled it into each MFE.

However, I agree that being able to pass in the Error Boundary in via custom props would be helpful and am sure other organizations would use it. One possible option would be to have single-spa-react look for a prop called errorBoundary and call that if it's present? See the code below:

https://github.com/single-spa/single-spa-react/blob/1abaf9df336872b148718ab70aaf81d2d415a7d8/src/single-spa-react.js#L361-L365

I think it would be as simple as changing it to the following:

const errorBoundary = opts.errorBoundary || props.errorBoundary
return errorBoundary(...)

Would that solution solve things for you? Have any other ideas about other ways to implement it? Would you have interest in contributing a pull request to implement this? I am transferring this over to the single-spa-react repo to track the implementation of this there.

joeldenning commented 3 years ago

I had some time tonight so I implemented this in #120

ahayes91 commented 3 years ago

@joeldenning That is awesome, quick work, thanks so much - looks like exactly what we would need. I'll keep an eye on the PR and release notes 😁