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

It's not possible to mount a micro-frontend if it expects a prop named `config` #195

Open fcano-ut opened 1 year ago

fcano-ut commented 1 year ago

The way the API is thought, it expects microfrontend props to be passed at the same level than the rest of the props..

This causes this code to fail if the microfrontend props contain a prop named "config"

<Parcel
  config={microfrontend}
  wrapWith="div"
  parcelDidMount={onMicrofrontendMounted}
  {...parcelProps}
/>

Of course, these conflicts doesn't only happen with config, but also with wrapWith and all the rest of the Parcel props, but the config case is the worst because it causes the Parcel not to render at all.

It seems like a solution to this, without introducing a breaking change, would be to have a new prop named something like parcelProps... If that prop is used, the props object to pass to the microfrontend could be that one, instead of the ones at the first level. It would be optional and if nothing is passed, it could continue working as before.

<Parcel
  config={microfrontend}
  wrapWith="div"
  parcelDidMount={onMicrofrontendMounted}
  parcelProps={parcelProps}
/>

Does this make sense? I think it should be a simple enough change, I could attempt to do it myself :) (Seems like the change is mostly on propTypes and here, and then of course the docs could need a change)

MilanKovacic commented 1 year ago

Hi, thank you for submitting the proposal. To me, proposal looks valid. Codemod could also be provided to migrate users to the new approach. @joeldenning, @robmosca what do you think?