4

I have created a custom modal hook that extending react-modal library, but I have a problem to work with inputs when it has onChange prop on the input, for some reason, it rerenders useModalHook component every time when input change happened. I am new to hooks, but is there a way to pass a hook wrapper that doesn't rerender the component and leaves it as is. My goals are to wrap any component or content that I pass through my custom modal so that it has a custom layout as I needed.

import * as React from "react";
import Modal from "react-modal";
import styles from "../styles.scss";

export const useModalHook = (
  {
    title = "",
    modalType = MODAL_TYPE.small,
    closeIcon = true,
  }
) => {
  const [ getOpenModal, setOpenModal ] = React
    .useState<boolean>(false);

  React.useEffect(() => {
    if (state) {
      setOpenModal(!state);
    }
    return () => setOpenModal(false);
  }, [ state ]);

  const handleOnRequestClose = (force = false) => setOpenModal(force);

  return {
    Modal: ({ children }) => getOpenModal && (
      <Modal
        isOpen={getOpenModal}
        onRequestClose={() => handleOnRequestClose()}
        contentLabel={title}
        appElement={document.getElementById("__next")}
      >
        <div className={styles.modalWrapper}>
          <div className={styles.modalTitle}>
            {title !== "" ? <h2>{title}</h2> : null}
            {closeIcon ? (
              <button
                className={styles.closeModal}
                onClick={() => setOpenModal(!getOpenModal)}
              >x</button>
            ) : null}
          </div>
          {children}
        </div>
      </Modal>
    ),
    closeModal: () => setOpenModal(false),
    toggleModal: () => setOpenModal(!getOpenModal),
    modalOpen: getOpenModal,
  };
};

Usage of the useModalHook in component


import * as React from "react";
import { useModalHook } from "components/UI/Hooks/useModalHook";

const Component = () => {
  const initialData = { Test: "" };

  const { Modal, toggleModal, modalOpen, closeModal } = useModalHook({
    title: "Cool Modal"
  });

  const handleOnChange = e => {
    return setData({
      ...getData,
      [ name ]: value
    });
  };

  React.useEffect(() => {
    setData(initialData);
  }, [ modalOpen ]);

  return (
    <>
      <button onClick={toggleModal}>Show</button>
      <Modal>
        <div className="modal_content">
          <input type="text" onChange={handleOnChange} value={getData.Test} name="Test"/>
        </div>
      </Modal>
    </>
  );
};

export default Component;

GYTO
  • 440
  • 1
  • 10
  • 23
  • 1
    In this case getOpenModal is NOT a function, so the name is misleading... – le3th4x0rbot Oct 20 '19 at 00:59
  • @trognanders it is a boolean – GYTO Oct 21 '19 at 14:11
  • Just a side note on hooks returning components. If you are making a form library, this is probably not the best approach. – Devin Rhode Dec 25 '20 at 19:20

2 Answers2

17

This is a tricky pattern that I also have not been able to find a "correct" answer for but I think I've got a reasonable solution.

What's happening is the following:

  1. <Component> renders, calling useModalHook
  2. useModalHook returns a new function component for <Modal> every time it runs
  3. React tears down the old <Modal> and all its children and replaces it with the new one

So every time a component using useModalHook renders it will tear down all the children of <Modal> and rebuild them because <Modal> is a "new" function component.

To prevent this you can do a couple things:

  1. Make sure you're returning the same function component from useModalHook as often as possible. This is what useMemo is good for.

  2. Move the definition of <Modal> outside of your hook then return any props it needs from useModalHook and apply them manually to <Modal> in the JSX of the component calling the hook. This is the "standard" react way to do it. Keep your logic in your hook and your JSX in a separate component. I don't like this solution for some cases because it introduces boilerplate and moves logic outside of the hook into a tightly coupled but still separate component (yuck). It forces you to repeat the application of props to <Modal> even though you're going to do it the same way every time.

  3. Move the logic out of a hook and make <Modal> its own top level component. Share your closeModal toggleModal with the parent via a ref. An ugly and discouraged pattern.

So to do #1 above, I would apply useMemo to preserve the instance of <Modal> as much as possible like so:


export const useModalHook = (
  {
    title = "",
    modalType = MODAL_TYPE.small,
    closeIcon = true,
  }
) => {
  const [ getOpenModal, setOpenModal ] = React
    .useState<boolean>(false);

  React.useEffect(() => {
    if (state) {
      setOpenModal(!state);
    }
    return () => setOpenModal(false);
  }, [ state ]);

   //useRef is good for preserving the identity of things -- without this handeOnRequestChange would be a new fn every render!
  const handleOnRequestClose = useRef((force = false) => setOpenModal(force)).current;

  const Modal = useMemo(() => {
      return ({ children }) => getOpenModal && (
      <Modal
        isOpen={getOpenModal}
        onRequestClose={() => handleOnRequestClose()}
        contentLabel={title}
        appElement={document.getElementById("__next")}
      >
        <div className={styles.modalWrapper}>
          <div className={styles.modalTitle}>
            {title !== "" ? <h2>{title}</h2> : null}
            {closeIcon ? (
              <button
                className={styles.closeModal}
                onClick={() => setOpenModal(!getOpenModal)}
              >x</button>
            ) : null}
          </div>
          {children}
        </div>
      </Modal>
    )
   //note that we list the dependencies here -- any time these change useMemo will
   //rerun and create a new component type resulting in a tear down and re-render
   //of <Modal> and its children
  }, [getOpenModal, handleOnRequestClose, title, closeIcon]}

  return {
    Modal,
    closeModal: () => setOpenModal(false),
    toggleModal: () => setOpenModal(!getOpenModal),
    modalOpen: getOpenModal,
  };
};

How well this works really depends on how often the dependencies change for your useMemo. It also depends on deeply understanding how react works and how equality works in JS.

IMO this is one of the few cases I've found where hooks doesn't offer a great pattern for reusing logic.

I've got a similar question here where you can find more detail on the topic Generating new component types in react hooks, how to maintain performance?

edit: Remember that if you use this pattern and any of the dependencies for your useMemo to create the <Modal> change then react will tear down the <Modal> and all children and lose all their state. You need to be sure this is OK or that this will not happen at times where it would cause disruption in order to use this pattern.

imagio
  • 1,390
  • 2
  • 16
  • 27
  • On my team, we made a hook called useFreeze/useFrozen/useStableRef which is literally just `useMemo` with an empty dependency array. (Since there's a specific reason for disregarding the eslint rule.) – Devin Rhode Feb 01 '21 at 20:48
3

I tried this idea before and found that the Modal component will rerender every time the Modal open or close.
I had the solution to use useModal hook without this issue by modify the useModal hook, we shouldn't return the Wrap Modal component , instead of that, we just return a useCallback function that return the Modal component with props and children are passed when we call that callback funtion.

And now, we can use useModal with less code and good performance:

useModal.js

import React, {useState, useCallback} from 'react'
import {Button, Modal as AntdModal} from 'antd'

const useModal = () => {
  const [on, setOn] = useState(false)
  const toggle = useCallback(() => setOn(!on), [on])
  const ModalBtn = props => <Button {...props} onClick={toggle} />
  const Modal = useCallback(
    ({onOK, ...rest}, child) => {
      return (
        <AntdModal
          {...rest}
          visible={on}
          onOk={() => {
            onOK && onOK()
            toggle()
          }}
          onCancel={toggle}
        >
          {child}
        </AntdModal>
      )
    },
    [on, toggle],
  )
  return {
    on,
    toggle,
    ModalBtn,
    Modal,
  }
}

export default useModal;

how to use it in index.js

mport React from 'react'
import ReactDOM from 'react-dom'
import useModal from './useModal'
import {Button} from 'antd'

const App = () => {
  const {ModalBtn, Modal} = useModal()
  return (
    <div>
      <ModalBtn type="primary">Click me!</ModalBtn>
      <Button type="danger">Danger</Button>
      {Modal( {title:"Simple", onOK:() => alert('everything is OK')},
        <>
        <p>Some contents...</p>
        <p>Some contents...</p>
        <p>Some contents...</p>
        </>)}
    </div>
  )
}

const rootElement = document.getElementById('root')
ReactDOM.render(<App />, rootElement)
leo mh
  • 31
  • 1
  • 1
    I think I'm concluding that the best thing to do is what you have done here. The only downside, I believe, is that you aren't invoking the `Modal` via ` – Devin Rhode Nov 27 '20 at 06:25
  • 1
    I agree with this, return JSX instead of a component. No extra cost when return JSX, the cost is the same as define this whole Modal inline in the parent component. (useCallback makes it a little better) – Morio Sep 02 '21 at 16:09