Fix multi-parcel unmount resolving

[pbowden91]

The root object currently only allows a single unmountFinished promise at a time. Multiple parcels unmounting at the same time leads to most promises orphaned and not resolving, throwing console warnings.

This PR uses the parcel name to differentiate unmount promises.

Before:

After:

[TheMcMurder]

The core library, single-spa, handles unmounting multiple parcels at once without an issue, I think there is a larger problem with the structure for single-spa react and while this change works it also changes the API in a way that I'm initially uncomfortable with because it changes it only for react and increases the mental surface area for maintenance.

[pbowden91]

Does it change the API? Seemed like unmountFinished was wholly internal, was trying to copy what was done w/ updateResolves.

[Edit] unless unmountFinished can be called without it being overwritten by unmount, but it's not in the external typings either

[pbowden91]

Ultimate issue: componentWillUnmount calls this.props.unmountFinished, which is defined here

single-spa's unmount call is here from single-spa

Current path, single parcel:

• single-spa unmount -> opts.unmountFinished is set to the current unmounting parcel's resolve -> when componentWillUnmount is finally called, that opts.unmountFinished is called so the promise is resolved "in a reasonable time"

Current path, parcels > 1:

• single-spa calls unmount on parcel 1
• opts.unmountFinished is set to parcel 1's resolve
• single-spa calls unmount on parcel 2
• opts.unmountFinished is set to parcel 2's resolve
• componentWillUnmount is called for parcel 1, which calls opts.unmountFinished - which is parcel 2's resolve
• componentWillUnmount is called for parcel 2, which calls opts.unmountFinished - which is parcel 2's resolve, which was already resolved
• After some time, single-spa throws warnings/errors that parcel 1 was never unmounted

We need componentWillUnmount to be guaranteed to resolve that parcel's unmount promise. Open to helping on another solution, shooting blind here but our error logs are getting bombarded and we want to make sure we aren't missing actual unmount issues.

[TheMcMurder]

The part of the API that appears to be changing is name. Naming a parcel isn't required in single-spa. This change requires using the name as a key which shifts name to a required field.

I could be missing where it's already required though, if it's already required please let me know.

[pbowden91]

Ah, it does seem name has a default, we don't set that either. All of ours end up as parcel-${id}.

Looks like the fallback happens here inside of the core library, and the parcel name gets set here.

Adjusting the fallback on that line locally does change all of the parcel names for me - e.g. I triggered the errors from the first gif and now the names all say my-auth-gen-parcel-name-{id} in the error URLs:

FWIW the file uses props.name throughout to reference the specific parcel - e.g. here or here in ways that appear breaking if name weren't defined.

For fun, I commented out name being assigned to see what else would break, and single-spa-css also fails:

[TheMcMurder]

Good clarification. Will review again with that insight. Thank you

[joeldenning]

Released in https://github.com/single-spa/single-spa-react/releases/tag/v5.1.0