Bug: Modal with closeTimeoutMS unrecoverable with Suspense

[h3rmanj]

Summary:

Passing closeTimeoutMS to an open Modal makes it unrecoverable if it's part of a render that suspends.

Steps to reproduce:

1. Create a component that contains an open modal, and something that can suspend
2. Suspend it after initial render
3. Modal can't recover

Expected behavior:

Modal should be open.

Link to example of issue:

https://codesandbox.io/s/closetimeoutms-and-suspense-nfmelh?file=/src/App.tsx

Additional notes:

If the closeTimeoutMS prop is removed from the codesandbox link, everything works as expected.

[diasbruno]

@h3rmanj This is super weird...why is it triggering the fallback if there is still content been displayed? (the h1 node)

[diasbruno]

It is the lazy component that triggers the suspense. Probably, suspense uses conditional rendering on their side and react-modal doesn't play well with it.

[diasbruno]

React-Modal: Cannot register modal instance that's already open in Unknown

🤔 it must be failing to remove the instance from the manager.

[h3rmanj]

Do note you have to reload the sandbox each time you try again, as it resolves and triggers the fallback only once per load.

Which h1 node are you referring? The developer I am opted to use it for all displayed elements 😅

In the docs it is noted that suspense will call layout effect cleanups, which might include componentDidUnmount etc.

Do note that this is a low-priority bug, which can be easily solved by localizing suspenses etc. But it was something we met, and used some time to understand why happened. I would think that the component would respect the isOpen prop no matter what.

[diasbruno]

My bad...I thought that the h1 tag in the sandbox would not allow to trigger the suspense, but actually it comes from the LazyComponent.

react-modal has a small manager to deal with nested modals and dangling references to modals.

In this case, since it doesn't trigger the logic to remove the instance from the manager,
it will fail when it tries to open a new instance.

You must be getting this log message:

React-Modal: Cannot register modal instance that's already open in Unknown

[h3rmanj]

Right, so if I remove closeTimeoutMS (codesandbox), what appears to be expected behavior is actually showing an old instance that should've been removed?

[diasbruno]

If suspense is been triggered, than it must create a new instance of the Modal, because that one was already "detroyed".

react-modal/src/helpers/portalOpenInstances.js

Lines 9 to 18 in 8a74268

	register = openInstance => {
	if (this.openInstances.indexOf(openInstance) !== -1) {
	if (process.env.NODE_ENV !== "production") {
	// eslint-disable-next-line no-console
	console.warn(
	`React-Modal: Cannot register modal instance that's already open`
	);
	}
	return;
	}

Unless react is doing some black magic to keep the previously rendered tree, and only append what is in "real suspense".

[h3rmanj]

If I understand the docs right, if the component has initially mounted, but then shows the fallback later, react will preserve the state of the components until the suspense promise is resolved.