Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot register modal instance that's already open #808

Open
Hainesy opened this issue May 28, 2020 · 38 comments · May be fixed by #1040
Open

Cannot register modal instance that's already open #808

Hainesy opened this issue May 28, 2020 · 38 comments · May be fixed by #1040

Comments

@Hainesy
Copy link

Hainesy commented May 28, 2020

When a modal is open, double-clicking on the background overlay results in the modal disappearing then quickly reappearing again with the following warning in the console:

react_devtools_backend.js:6 React-Modal: Cannot register modal instance that's already open 
    at ModalPortal (http://localhost:4500/static/js/0.chunk.js:60569:5)
    at Modal (http://localhost:4500/static/js/0.chunk.js:60212:5)

Additionally, the ReactModal__Body--open class remains on the body after closing the re-opened modal.

@Hainesy
Copy link
Author

Hainesy commented May 30, 2020

Managed to get around the issue by adding pointer-events: none to .ReactModal__Overlay--before-close.

@diasbruno
Copy link
Collaborator

diasbruno commented May 30, 2020

@Hainesy

[edit]
Thought we had a handler for double-click on the overlay. So, once we have that it should do only one action executed.

@rahul-desai3
Copy link

I am getting same error even after setting pointer-events: none as mentioned above

@tonhao-dev
Copy link

tonhao-dev commented Jan 3, 2021

When I click once outside the modal to close it I get:
React-Modal: Cannot register modal instance that's already open

and when I click outside the modal again I get:
portalOpenInstances.js:33 React-Modal: Unable to deregister [object Object] as it was never registered

and then the modal is closed

@Hainesy
Copy link
Author

Hainesy commented Jan 7, 2021

I agree this is still a problem

@timothymalcham
Copy link

I am now getting this error after upgrading to React 18. Prior to doing the upgrade, I did not see this error: React-Modal: Cannot register modal instance that's already open. Does anybody have any idea of what might have changed?

@pioferen
Copy link

This issue still occurs, I'm using version 3.5.1

@diasbruno
Copy link
Collaborator

Version 3.15.1 was released. Is this happening on this version?

@pioferen
Copy link

Version 3.15.1 was released. Is this happening on this version?

Yes, but I think it occurs only in development mode with react 18 and enabled StrictMode reactwg/react-18#19

@diasbruno
Copy link
Collaborator

Hmmm...this is interesting...could you please help creating a reproducible example?

@pioferen
Copy link

Hmmm...this is interesting...could you please help creating a reproducible example?

here: https://stackblitz.com/edit/nextjs-aznh2l?file=pages%2Findex.js

without modalIsOpen && error doesn't occur

@diasbruno
Copy link
Collaborator

Hmmmm...It's recommended to not use modal with conditional rendering. There is a problem with createPortal when using this way (need to check if it still happening. It starts happening on version +16.3 of react).
Are you all using conditional rendering?

@diasbruno
Copy link
Collaborator

cc @Hainesy @timothymalcham @rahul-desai3 @luis-antonio-dev

@Brhernann
Copy link

Same warning

image

@deep-cognite
Copy link

deep-cognite commented May 29, 2022

Facing the same issue for one of my modals after upgrading to React 18. Any fixes to this?
Funnily enough, the modal starts working "after a while" (I think when I leave the app and come back). Super strange

@lassonico
Copy link

¿En definitiva no hay solucion?

@diasbruno
Copy link
Collaborator

Please read this to help debugging this issue

It's recommended to not use modal with conditional rendering. There is a problem with createPortal when using this way (need to check if it still happening. It starts happening on version +16.3 of react).
Are you all using conditional rendering?

A suggestion is: don't use conditional rendering with modals...there is no need to. There is no performance cost, there is no problem to maintain (it's only one state).

Sorry for this guys... 😂
But please let me know if you have a different case where this happens other than with conditional rendering.

@EGurney
Copy link

EGurney commented Jun 28, 2022

Please read this to help debugging this issue

It's recommended to not use modal with conditional rendering. There is a problem with createPortal when using this way (need to check if it still happening. It starts happening on version +16.3 of react). Are you all using conditional rendering?

A suggestion is: don't use conditional rendering with modals...there is no need to. There is no performance cost, there is no problem to maintain (it's only one state).

Sorry for this guys... 😂 But please let me know if you have a different case where this happens other than with conditional rendering.

This solved the issue for me.

I was conditionally rendering in my component, rather than letting the isOpen prop handle the rendering.

@rodriguezmarting
Copy link

The bug also happens when the Modal lives next to a component that conditionally renders.
Plus it only happens when using base | afterOpen | beforeClose classNames.

Something like this won't work:

export const MainHeader2 = () => {
  const { isMobile } = useContext(ViewportContext);

  const [isOpen, setIsOpen] = useState(false);

  const toggleConfirmModalOrDrawer = () => {
    setIsOpen((prevState) => !prevState);
  };

  const doSomething = () => someAction();

  return (
    <header>
      <Logo handleClick={toggleConfirmModalOrDrawer} />
      <Modal
        className={{
          base: styles.modalContent,
          afterOpen: styles.modalContentAfterOpen,
          beforeClose: styles.modalContentBeforeClose,
        }}
        overlayClassName={styles.modalOverlay}
        isOpen={!isMobile && isOpen}
        onRequestClose={toggleConfirmModalOrDrawer}
      >
        <ConfirmLeaveModalContent
          onConfirm={doSomething}
          onDismiss={toggleConfirmModalOrDrawer}
        />
      </Modal>
      {isMobile && (
        <ConfirmLeaveDrawer
          onConfirm={doSomething}
          onDismiss={toggleConfirmModalOrDrawer}
        />
      )}
    </header>
  );
};

@diasbruno
Copy link
Collaborator

@rodriguezmarting Awesome. This is weird...
Can you make a step to reproduce so we can see how are you triggering this behavior?

@oroszi1mark
Copy link

Using react-modal with nice-modal-react makes for a handy combination in my opinion, but unfortunately nice-modal-react also uses conditional rendering. The console warning can be triggered in this example.

@Phanabani
Copy link

Phanabani commented Aug 17, 2022

I was having this error (react-modal@3.15.1, React@18.2.0), and after a lot of frustration found out that my problem was caused by using React.StrictMode in my root element, which apparently double-invokes some lifecycle methods in development mode (not production, though). As of now, this behavior is documented in the StrictMode docs -- search for "double-invoking the following functions".

I found this solution by accident, because I was investigating an effect being called twice and found this answer, which also referenced this answer which has more info about the double rendering of StrictMode.

It seems like this is harmless in my case because the only visible issue is the warning in console (modals are still displaying fine), but if anyone has a suggestion for suppressing the warning, I'd appreciate it! Removing the StrictMode tag stops this issue from happening, but I want to keep the benefits of using strict mode.

@mleister97
Copy link

Please fix this issue!!!

@diasbruno
Copy link
Collaborator

Why don't you fix it, @mleister97?

@honeymaro
Copy link

I have the same issue.

no conditional rendering,
no strict mode.

react-modal 3.16.1,
react 18.2.0

@diasbruno
Copy link
Collaborator

diasbruno commented Jan 10, 2023

This seems to be a mem leak on the src/helpers/portalOpenInstances.js, but it's most likely that this is caused by something that is holding the reference of a modal that should already be gone.

honeymaro added a commit to dezswap/dezswap-web-app that referenced this issue Jan 11, 2023
honeymaro added a commit to dezswap/dezswap-web-app that referenced this issue Jan 11, 2023
@rodriguezmoradiego15
Copy link

We Can't use:
{modalIsOpen && (......)
The best way to use that is creating a component where we have all the code that we need, a componet seems something like this:

<Modal isOpen={modal} style={customStyles} > <ModalProducto /> </Modal>
and ModalProducto.jsx is something like this:
export default function ModalProducto() { return ( <div>ModalProducto</div> ) }

That way works for me

No podemos usar:
{modalIsOpen && (......)
Lo que se debe hacer es crear un componente con toda la lógica que necesitamos, algo parecido a lo siguiente:
<Modal isOpen={modal} style={customStyles} > <ModalProducto /> </Modal>

y ModalProducto.jsx es algo como lo siguiente:
export default function ModalProducto() { return ( <div>ModalProducto</div> ) }
Esa es la solución.

@voldemote
Copy link

This error is occurring because you haven't declared the CSS for the modal. If you declare the CSS in a global file such as index.css, the error will not happen.

.mymodal {
position: fixed;
top: 50%;
left: 50%;
transform: translate(-50%, -50%);

border: 1px solid #ccc;
background: #fff;
overflow: auto;
border-radius: 4px;
outline: none;
padding: 20px;
}

.myoverlay {
position: fixed;
top: 0;
left: 0;
right: 0;
bottom: 0;
background-color: rgba(0, 0, 0, 0.75);
}

.ReactModal__Overlay {
opacity: 0;
transition: opacity 500ms ease-in-out;
}

.ReactModal__Overlay--after-open {
opacity: 1;
}

.ReactModal__Overlay--before-close {
opacity: 0;
}

@brianpeiris
Copy link

brianpeiris commented Feb 26, 2023

If it helps, I was able to reproduce this issue with minimal changes to the simple_usage example in this repo.

The steps I took:

  1. Upgrade to React 18
  2. Add React.StrictMode to the example
  3. Use createRoot instead of ReactDOM.render
  4. Set isOpen to true by default

The warning then appears in the console when you open the example.

Here is the diff: https://github.com/reactjs/react-modal/compare/master...brianpeiris:react-modal:register-warning?w=1

I did not have to use conditional rendering to reproduce this issue, and more people will run into it as they adopt React 18. Notably, modern tooling, like Vite, enables StrictMode by default.

@diasbruno
Copy link
Collaborator

@brianpeiris Yep, this is not just a case of conditional rendering (but kinda related).

The problem seems to be a new feature of react 18 where it can keep some components alive to make some rendering optimizations. But this causes all sorts of problems because react-modal is a stateful component that needs to deal with subtrees.

@diasbruno
Copy link
Collaborator

I'm flagging this as hardcore. I did a little investigation, don't have time to fully jump into this, and, this is super annoying.

It's all about the new react version and this component src/components/Modal.js, specially when the component is unmounted.

If you want to be at the 1% top react dev, this is a great challenge.

You need to:

  • Check the places where we (de)register the modal (mount and unmount)
  • Make sure react calls unmount when it is supposed to (this is actually a problem since react 16.3+)

This is enough information to start digging in.

If anyone wants to fight with this, let me know. For any other questions, I'll be here to help.

@Hebzebba
Copy link

Hebzebba commented Mar 5, 2023

Version 3.15.1 was released. Is this happening on this version?

Yes, but I think it occurs only in development mode with react 18 and enabled StrictMode reactwg/react-18#19

Disabling StrictMode fix the issue for me

@berryboylb
Copy link

I don't

I'm flagging this as hardcore. I did a little investigation, don't have time to fully jump into this, and, this is super annoying.

It's all about the new react version and this component src/components/Modal.js, specially when the component is unmounted.

If you want to be at the 1% top react dev, this is a great challenge.

You need to:

  • Check the places where we (de)register the modal (mount and unmount)
  • Make sure react calls unmount when it is supposed to (this is actually a problem since react 16.3+)

This is enough information to start digging in.

If anyone wants to fight with this, let me know. For any other questions, I'll be here to help.

I don't want to be a top react dev, just fix the issue please.

@diasbruno
Copy link
Collaborator

@berryboylb 😿

@shulcsm
Copy link

shulcsm commented Apr 26, 2023

Issue is here:
https://github.com/reactjs/react-modal/blob/master/src/components/ModalPortal.js#L136

When strict mode unmounts isOpen is not yet initialized so it's not de-registering the modal correctly so it leaks instances and focus handlers.

This "works"

  if (this.state.isOpen || (typeof this.state.isOpen === 'undefined' && this.props.isOpen)) {
    this.afterClose();
  }

But honestly whole thing should be rewritten.

@diasbruno
Copy link
Collaborator

@shulcsm You can rewrite if you want. We will be really happy.

This library remains working on really dated versions of react to maintain backwards-compatibility. It's great, but also has some drawback...

Feel free to share some ideas regarding this.

@doeg
Copy link

doeg commented Feb 11, 2024

Hey @diasbruno, want me to take this one? Feel free to assign it to me if so. 🙇

honeymaro added a commit to honeymaro/make-react-modal-great-again that referenced this issue Feb 13, 2024
- Fixed [reactjs#808](reactjs#808)
- Checking `this.props.isOpen` instead of `this.state.isOpen` in `componentWillUnmount`.
- Moved the call to `this.beforeOpen()` from the beginning of the `open` method to an else block.
- Set `this.closeTimer` without waiting until the end of the setState.
- Call `this.afterClose()` directly after setting the state in `closeWithoutTimeout`.
@honeymaro honeymaro linked a pull request Feb 13, 2024 that will close this issue
2 tasks
@ADTC
Copy link
Contributor

ADTC commented Apr 5, 2024

This happened to me, not because of conditional rendering, but because I was forcing the modal to be already open upon page load:

  const [isOpen, setIsOpen] = useState(true)

The fix is to use an effect hook to open the modal after the component has loaded:

  const [isOpen, setIsOpen] = useState(false)

  // Fix for warning "Cannot register modal instance that's already open".
  // The modal should only be opened after the component has mounted.
  useEffect(() => setIsOpen(true), [])

This will help force the modal to be open as soon as the page loads, but avoids the warning "Cannot register modal instance that's already open".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.