Fix #1184 #1189
Fix #1184 #1189
Conversation
|
G'day team, I'm keen to get a code review on this. I reckon it's something that ought to be sorted by the single-spa library, maybe right here or over in single-spa-react/parcel. I've set up a repo showing the fix in action, took a leaf out of the book from this pattern to make the change. Just holler if there's anything else needed on my end. |
|
@joeldenning, what's your take on this solution? |
|
@MilanKovacic , what's your take on this solution? |
|
Hi, thank you for submitting the pull request. I will take a look. |
There was a problem hiding this comment.
The code change looks reasonable, I've left a couple comments.
Are you someone who I've asked not to contact me @BeastlyUnsold? I ask because issue #1184 was created by an Adobe employee, and I have been harassed by a dev at adobe who works on single-spa stuff. Your Github account was created recently and only has worked on this Adobe issue, which is enough for me to wonder whether I'm being alted by someone I've asked to never contact me again. I am cautious about it because he hasn't been able to stop interacting with me despite repeated requests and even legal action.
| parcelConfig.unmount = () => { | ||
| parcelConfig.unmountCalls++; | ||
| return new Promise((r) => { | ||
| setTimeout(r, 2000); |
There was a problem hiding this comment.
Why wait 2 seconds to resolve the promise?
| }); | ||
| // In some cases it is possible for a parcel to be in an unmounting status when `unmountThisParcel` is called a second time. | ||
| // https://github.com/single-spa/single-spa/issues/1184 | ||
| if (parcel.status === UNMOUNTING && !!parcel.internalUnmountPromise) { |
There was a problem hiding this comment.
Truthiness check is good enough for this if statement, no?
| if (parcel.status === UNMOUNTING && !!parcel.internalUnmountPromise) { | |
| if (parcel.status === UNMOUNTING && parcel.internalUnmountPromise) { |
| }) | ||
| .then((value) => { | ||
| resolveUnmount(value); | ||
| return value; |
There was a problem hiding this comment.
Deleting the internalMountPromise might be helpful for garbage collection.
| return value; | |
| delete this.internalMountPromise; | |
| return value; |
Fix #1184
Proof of issue: https://codesandbox.io/p/sandbox/vigilant-jepsen-gkznvg