ionic-framework: bug: Conditionally rendering ionModal in react - Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x
  • Nightly

Current Behavior

Conditionally rendering an IonModal, breaks when dismissing when using ionic v6.

Observed behavior: white/blank screen.

Error in console: react-dom.development.js:10301 Uncaught DOMException: Failed to execute ‘removeChild’ on ‘Node’: The node to be removed is not a child of this node.

Expected Behavior

In an ideal scenario - the ionic modal could be conditionally rendered (without using the isOpen prop) this is a common pattern in react applications.

Steps to Reproduce

  1. Conditionally render an IonModal
...
const [showIonModal, setShowIonModal] = useState(false);
...

return (
<div>
  <IonButton onClick={() => setShowIonModal(true)}>
    Show Modal
  </IonButton>
  {showIonModal && (
    <IonModal isOpen={true} onDidDismiss={() => setShowIonModal(false)}>
      <IonHeader>
        <IonToolbar>
          <IonTitle>Content</IonTitle>
        </IonToolbar>
      </IonHeader>
      <IonContent fullscreen>
        Ionic Modal Content here!
        <IonButton color="danger" onClick={() => setShowIonModal(false)}>
          Hide Modal (will break)
        </IonButton>
      </IonContent>
    </IonModal>
  )}
</div>
)
...
  1. Dismiss the modal (so that IonModal is pulled from the dom)
  2. Observe the error.

Code Reproduction URL

https://github.com/corysmc/react-overlay-bug

Ionic Info

Ionic:

Ionic CLI : 6.15.0 Ionic Framework : @ionic/react 6.1.13

Utility:

cordova-res : not installed native-run : 1.6.0

System:

NodeJS : v14.16.0 npm : 6.14.11 OS : macOS Big Sur

Additional Information

Related: https://github.com/ionic-team/ionic-framework/issues/24887

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Comments: 50 (27 by maintainers)

Most upvoted comments

Once I try to use @ionic/react@6.2.3-dev.11660337759.18ea0f7e navigation is broken. Downgrading to 6.2.2 router works fine - but the issue outlined above is still present.

Tapping on a navigation link, changes the URL, then refreshes the app back to the root url

The same thing happened when I downloaded the react conference app - and upgraded to this version of ionic/react.

Hello! Just wondering when an update will be made to correct this. Thanks!

@JohnGeorgiadis perfect, thank you for the reproduction!

Here is an updated dev-build that I tested against your reproduction:

npm install @ionic/react@6.2.8-dev.11663187107.1a1cbb3f @ionic/react-router@6.2.8-dev.11663187107.1a1cbb3f

When presenting any overlay, we are now re-assigning the z-index value to make it the top overlay. Previously we didn’t do this because inline overlays were being reconstructed when presented. We need to vet the impact of this change (appreciate any extra testing that overlays work correctly in your apps!), but if this change lands as-is, should be a minor performance improvement to inline overlays, since it reuses the DOM node and only does a style re-calc for the z-index change.

Hello @sean-perkins https://github.com/JohnGeorgiadis/ion-datetime-button-issue here it is. I can still reproduce it.

I am loading a modal on tab1 with state isOpen. Screenshot 2022-09-14 at 09 43 14

Modal is opened with a datetimebutton Screenshot 2022-09-14 at 09 43 20

I cannot see the picker when I click on the modal Screenshot 2022-09-14 at 09 43 26

Maybe I am missing something obvious here.

@corysmc the behavior of the modal remaining presented when using back navigation appears to be an issue reproducible in main and is not isolated to the dev-build.

We will need to report that as a new issue, so it can be triaged and prioritized in our backlog.

If that is the only outstanding issue here, I believe the dev-build still resolves the original reported issue.

@sean-perkins I had the same routing issue with this build as @Fooweb and @JohnGeorgiadis reported but the issue I saw with routing was because I needed to install two dev versions to test:

@ionic/react@6.2.3-dev.11660337759.18ea0f7e and @ionic/react-router@6.2.3-dev.11660337759.18ea0f7e

This build is working much better - however there’s still an edge case that I have a hack fix for. When navigating away from a route that renders a modal - the modal stays open. So this is what I’ve done to “fix” it.

import React, { ComponentProps, useLayoutEffect, useRef } from 'react';
import { IonModal } from '@ionic/react';

type TnModalProps = ComponentProps<typeof IonModal>;

const TnModal: React.FC<TnModalProps> = (props) => {
  const { children, ...ionModalProps } = props;
  const modalRef = useRef<HTMLIonModalElement>(null);

  useLayoutEffect(
    () => () => {
      modalRef.current?.dismiss();
    },
    []
  );

  return (
    <IonModal ref={modalRef} {...ionModalProps}>
      {children}
    </IonModal>
  );
};

export default TnModal;

Another issue popped up that we believe to be related to the problematic behavior here: https://github.com/jameson-w-taylor/modal-state-error

Rendering new child nodes before an inline modal, during the modal dismiss, causes the same exception as originally reported.

Over simplified code:

{items.map(item => <IonCard />)}
<IonModal ref={modal} onWillDismiss={() => {
  setItems(...items, 'Another one');
}}>
  <IonButton onClick={() => modal.current.dismiss()}>Close</IonButton>
</IonModal>

FYI we do consider this a bug, but leaving in triage status just to keep it top-of-queue to try and get a better grasp of where the root problem exists.

@sean-perkins I understand that useIonModal is the way around the issue, however - this is a very common and preferred pattern for react applications. When using the hook useIonModal - in a full scale application you end up having to wrap every modal hook with providers to ensure it works properly. When using a conditionally rendered modal - all the providers are there.

Also, take a look at the linked repo - the dismiss animation still works because it’s called in the useLayoutEffect callback that is called before unmounting 😁

https://github.com/corysmc/react-overlay-bug/blob/development/src/components/CustomModal.tsx

@corysmc thanks for opening this issue!

I tried to recreate a similar example in Angular to see what the behavior was there. While it doesn’t error out, it does highlight a few problematic side effects of conditionally rendering modals in this way.

  1. There is no dismiss animation for the modal.
  2. None of the dismiss behavior fires (no dismiss events are emitted).

Since the framework is performing the remove operation, a lot of internal behavior is skipped. Modal is not currently architected to perform expected dismiss behavior when the node is removed outside of calling dismiss() or performing specific actions.

I’ll need to compare with React and another modal library implementation to see how they handle this. I’ll also need to explore useIonModal more and see if that’s a more applicable pattern for achieving this behavior.