Using reference for parentSelector causes "Cannot read property 'ownerDocument' of null" error

[tuckbarber]

Summary:

I believe I found a bug the happens when the parentSelector prop function is set by reference, in cases where the parentSelector is an element within the page, and a parent is unmounted suddenly.

This is documented and addressed before (here , and here), but I think I am running into a wall caused by a since added update

Since I am passing the parent node via function return

parentSelector={ () => modalParentRef.current} 

the new check

(parentSelector && parentSelector().ownerDocument) || document

throws an error "Cannot read property 'ownerDocument' of null" when the existing node referenced by modalParentRef.current is removed (because of navigation or parent unmount). This is because parentSelector is still a defined function, but returns null.

I have tried a number of workarounds, such as

parentSelector={
    modalParentRef.current
        ? () => modalParentRef.current
        : undefined
}

but I think the prop does not update in time.

My current workaround is

parentSelector={
    (modalParentRef.current || { ownerDocument: undefined, contains: () => false }) as HTMLElement
}

which is unseemly at best, but it works as I would expect.

Steps to reproduce:

1. Use a Ref in the parentSelector prop e.g. parentSelector={() => someRef.current}

2. Unmount a parent containing a modal

3. Get crash and "Cannot read property 'ownerDocument' of null" error

Expected behavior:

I would expect the Modal to realize the return node no longer exists, and handle it that way. My simple and humble suggestion only based on #965 would be to check something like

(parentSelector && parentSelector() && parentSelector().ownerDocument) || document

but there may be a better fix

[diasbruno]

@tuckbarber In this case it's not a bug. If you use parentSelector, then it must always return a valid element.

We could:

1. display a warning about it (not sure if it is the right way).
2. fail explaining what happened
3. update the doc to explicitly say that it must always return a valid element (seem to be the way to go)

[tuckbarber]

Thanks for the reply @diasbruno That makes sense.

I wish there were some way to unmount this modal's parent safely. I think the issue is that the Modal is trying to remove the portal via the now non-existant parent specified by the ref, yeah?

I find something like

parentSelector={
    () => (props.modalParentRef.current ? props.modalParentRef.current : document) as HTMLElement
}

also fails (Failed to execute 'removeChild') because the document obviously doesn't contain the portal node it is looking for either.

[diasbruno]

hmmm...It'll depend on your setup and/or the lifetime of the parent element.

You should be fine by rearranging things...or it also can be caused by redirecting too early,
like performing a redirect where you set the modal state to close
or on the onRequestClose close.

[tuckbarber]

@diasbruno Aha your rearranging comment along with facebook/react#14811 (comment) gave me the idea to stick with the general ordering of

<div>
    <ReactModal parentSelector={selectorFunction} />
    <div ref={parentRef} />
</div>

with the modal first. This was previously giving me an error with 'appendChild'- instead of 'removeChild' as when the order was parent, then modal. The key was something like

parentSelector={ modalParentRef.current ? () => modalParentRef.current : undefined }

So that the parent is only used after it is set. This seems to work without errors or warnings. Thanks!

[diasbruno]

This will "reparent" the modal to the default we use. It's kinda ok tough.
Just be careful with "reparenting", it may cause some leaks...

[diasbruno]

I'll close this, but let me know if you still have problems with this.