-
Notifications
You must be signed in to change notification settings - Fork 808
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
Comments
Managed to get around the issue by adding |
[edit] |
I am getting same error even after setting |
When I click once outside the modal to close it I get: and when I click outside the modal again I get: and then the modal is closed |
I agree this is still a problem |
I am now getting this error after upgrading to React 18. Prior to doing the upgrade, I did not see this error: |
This issue still occurs, I'm using version 3.5.1 |
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 |
Hmmm...this is interesting...could you please help creating a reproducible example? |
here: https://stackblitz.com/edit/nextjs-aznh2l?file=pages%2Findex.js without |
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). |
cc @Hainesy @timothymalcham @rahul-desai3 @luis-antonio-dev |
Facing the same issue for one of my modals after upgrading to React 18. Any fixes to this? |
¿En definitiva no hay solucion? |
Please read this to help debugging this issueIt'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). 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... 😂 |
This solved the issue for me. I was conditionally rendering in my component, rather than letting the |
The bug also happens when the Modal lives next to a component that conditionally renders. 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>
);
}; |
@rodriguezmarting Awesome. This is weird... |
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. |
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 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 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 |
Please fix this issue!!! |
Why don't you fix it, @mleister97? |
I have the same issue. no conditional rendering, react-modal |
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. |
We Can't use:
That way works for me No podemos usar: y ModalProducto.jsx es algo como lo siguiente: |
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 { border: 1px solid #ccc; .myoverlay { .ReactModal__Overlay { .ReactModal__Overlay--after-open { .ReactModal__Overlay--before-close { |
If it helps, I was able to reproduce this issue with minimal changes to the The steps I took:
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. |
@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. |
I'm flagging this as It's all about the new react version and this component If you want to be at the 1% top react dev, this is a great challenge. You need to:
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. |
Disabling StrictMode fix the issue for me |
I don't
I don't want to be a top react dev, just fix the issue please. |
Issue is here: When strict mode unmounts This "works" if (this.state.isOpen || (typeof this.state.isOpen === 'undefined' && this.props.isOpen)) {
this.afterClose();
} But honestly whole thing should be rewritten. |
@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. |
Hey @diasbruno, want me to take this one? Feel free to assign it to me if so. 🙇 |
- 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`.
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". |
Conditional rendering causes the problem, true. you can use dynamic (react or next) |
My god what an entitled prick! Make me a coffee, now! |
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:
Additionally, the
ReactModal__Body--open
class remains on the body after closing the re-opened modal.The text was updated successfully, but these errors were encountered: