Search code examples
reactjsreact-hooksweb-component

React useEffect cleanup function not undoing changes caused by React strict mode in development


This issue references to the fact that React intentionally remounts your components in development to find bugs.

I know I can turn off strict mode, but React doesn't recommend it. I'm just looking for a recommended approach to fix this.

I'm trying to build an Open Modal Component that closes either when the close button is clicked, or the user clicks outside the modal contents.

So, to implement the outside click closing of the modal, I'm using a useEffect hook that I want its callback function to only run when the isModalPopedUp state changes, and won't run on the initial render of the component. For this I use a custom hook:


import { useRef } from "react";

import { useEffect } from "react";

// This custom hook sets if a useEffect callback runs at initial render or not
export default function useDidMountEffect(argObject = {}) {
    /* 
    argObject = {
        runOnInitialRender: boolean,
        callback: () => void,
        dependencies: any[] // array of dependencies for useEffect
    }
    */
    const didMountRef = useRef(argObject.runOnInitialRender);

    // useEffect to run
    useEffect(() => {
        if (didMountRef.current) {
            // callback will run on first render if argObject.runOnInitialRender is true
            argObject.callback();
        } else {
            // callback will now run on dependencies change
            didMountRef.current = true;
        }
    }, argObject.dependencies); // only run if dependencies change
}

For the modal component, I split it into two components, the parent-modal that handles state logic:


import { useEffect, useState } from "react";
import Modal from "./modal";
import useDidMountEffect from "./useDidMountHook";
import "./modal.css";

export default function ModalParent() {
    const [isModalPopedUp, setIsModalPopedUp] = useState(false);

    function handleToggleModalPopup() {
        setIsModalPopedUp((prevState) => !prevState);
    }

    function handleOnClose() {
        setIsModalPopedUp(false);
    }

    function handleOutsideModalClick(event) {
        !event.target.classList.contains("modal-content") && handleOnClose();
    }

    // add event listener for outside modal click using the useDidMountEffect hook
    useDidMountEffect({
        runOnInitialRender: false,
        callback: () => {
            // add event listener when modal is shown
            if (isModalPopedUp) {
                document.addEventListener("click", handleOutsideModalClick);
            } else {
                // remove event listener when modal is closed
                document.removeEventListener("click", handleOutsideModalClick);
            }

            // return a cleanup function that removes the event listener when component unmounts
            return () => {
                document.removeEventListener("click", handleOutsideModalClick);
            };
        },
        dependencies: [isModalPopedUp], // only re-run the effect when isModalPopedUp changes
    });

    return (
        <div>
            <button
                onClick={() => {
                    handleToggleModalPopup();
                }}>
                Open Modal Popup
            </button>
            {isModalPopedUp && (
                <Modal
                    header={<h1>Customised Header</h1>}
                    footer={<h1>Customised Footer</h1>}
                    onClose={handleOnClose}
                    body={<div>Customised Body</div>}
                />
            )}
        </div>
    );
}

and the main modal component:


export default function Modal({ id, header, body, footer, onClose }) {
    return (
        <div id={id || "modal"} className="modal">
            <div className="modal-content">
                <div className="header">
                    <span onClick={onClose} className="close-modal-icon">
                        &times; {/* an X icon */}
                    </span>
                    <div>{header ? header : "Header"}</div>
                </div>
                <div className="body">
                    {body ? (
                        body
                    ) : (
                        <div>
                            <p>This is our Modal body</p>
                        </div>
                    )}
                </div>
                <div className="footer">
                    {footer ? footer : <h2>footer</h2>}
                </div>
            </div>
        </div>
    );
}

So, the problem is that React remounts the parent component after initial render, making the callback for the useDidMountEffect to immediately add the click event listener to the document element without having the isModalPopedUp state being changed to true by the "open modal" button click.

So, when clicking on the "open modal" button, isModalPopedUp is toggled to true but then is immediately changed to false due to the click event listener being added to the document prematurely. So, it eventually makes the modal unable to be opened by clicking the "open modal" button.

React.dev gives an easy fix by using the cleanup function returned by the useEffect callback to undo the changes from the remounting:

Usually, the answer is to implement the cleanup function. The cleanup function should stop or undo whatever the Effect was doing. The rule of thumb is that the user shouldn’t be able to distinguish between the Effect running once (as in production) and a setup → cleanup → setup sequence (as you’d see in development).

My cleanup function removes the click event listener from the document element, but it still doesn't fix the problem.

For the styling of the modal, I'm using:


/** @format */

.modal {
    position: fixed;
    z-index: 1;
    padding-top: 2rem;
    left: 0;
    top: 0;
    width: 100%;
    height: 100%;
    overflow: hidden;
    background-color: #b19b9b;
    color: black;
    overflow: auto;
    padding-bottom: 2rem;
}

.modal-content {
    position: relative;
    background-color: #fefefe;
    margin: auto;
    padding: 0;
    border: 1px solid red;
    width: 80%;
    animation-name: animateModal;
    animation-duration: 0.5s;
    animation-timing-function: ease-in-out;
}

.close-modal-icon {
    cursor: pointer;
    font-size: 40px;
    position: absolute;
    top: 0.5rem;
    right: 0.5rem;
    font-weight: bold;
}

.header {
    padding: 4px 10px;
    background-color: #5cb85c;
    color: white;
}

.body {
    padding: 2px 16px;
    height: 200px;
}

.footer {
    padding: 4px 16px;
    background-color: #5cb85c;
    color: white;
}

@keyframes animateModal {
    from {
        top: -200px;
        opacity: 0;
    }

    to {
        top: 0;
        opacity: 1;
    }
}


Solution

  • Based on the guidelines given by @justnut and @Mr. Hedgehog, I was able to come up with a working solution.

    The issue is caused by the click event listener being set on the document element prematurely when the "open modal" button is clicked (due to the way the useEffect hook works).

    So, when the "open modal" button is clicked, the event bubbles/propagates up to the document element, thereby unexpectedly triggering the handOutsideClick function.

    The fix is therefore to stop the event propagation up to the document element by using the event.stopPropagation() method in the callback of the "open modal" button click event.

    For checking whether the click was outside the modal, I used @justnut approach. Based on @Mr. Hedgehog explanations, I returned a better cleanup function in the useEffect hook.

    This is the overall modified code in the parent-modal component (the useDidMountEffect discarded):

    
    /** @format */
    
    import { useEffect, useRef, useState } from "react";
    import Modal from "./modal";
    import "./modal.css";
    
    export default function ModalParent() {
        const [isModalPopedUp, setIsModalPopedUp] = useState(false);
    
        function handleToggleModalPopup() {
            setIsModalPopedUp((prevState) => !prevState);
        }
    
        function handleOnClose() {
            setIsModalPopedUp(false);
        }
    
        function handleClickOutside(event) {
            if (modalRef.current && !modalRef.current.contains(event.target))
                handleOnClose();
        }
    
        // create modalRef for handling outside modal click events
        const modalRef = useRef(null);
    
        // create a useEffect for handling outside modal click events
        useEffect(() => {
            if (isModalPopedUp) {
                window.addEventListener("click", (event) => {
                    handleClickOutside(event);
                });
            } else {
                // Remove the event listener when modal is closed
                window.removeEventListener("click", handleClickOutside);
            }
    
            // Cleanup: Remove the event listener when component unmounts
            return () => {
                window.removeEventListener("click", handleClickOutside);
            };
        }, [isModalPopedUp]);
    
        return (
            <div>
                <button
                    onClick={(event) => {
                        event.stopPropagation();
                        // toggle modal popup state
                        handleToggleModalPopup();
                    }}>
                    Open Modal Popup
                </button>
                {isModalPopedUp && (
                    <Modal
                        header={<h1>Customised Header</h1>}
                        footer={<h1>Customised Footer</h1>}
                        onClose={handleOnClose}
                        body={<div>Customised Body</div>}
                        modalRef={modalRef}
                    />
                )}
            </div>
        );
    }
    
    

    and a little addition to the main modal component where I added the modalRef prop:

    
    /** @format */
    
    export default function Modal({ id, header, body, footer, onClose, modalRef }) {
        return (
            <div id={id || "modal"} className="modal">
                <div className="modal-content" ref={modalRef}>
                    <div className="header">
                        <span onClick={onClose} className="close-modal-icon">
                            &times; {/* an X icon */}
                        </span>
                        <div>{header ? header : "Header"}</div>
                    </div>
                    <div className="body">
                        {body ? (
                            body
                        ) : (
                            <div>
                                <p>This is our Modal body</p>
                            </div>
                        )}
                    </div>
                    <div className="footer">
                        {footer ? footer : <h2>footer</h2>}
                    </div>
                </div>
            </div>
        );
    }
    
    

    Help: If anyone can turn this into a runnable react stack snippet, it'd much appreciated. The modal component and css for styling is in the question above.