1

The docs explicitly state this:

Avoid using refs for anything that can be done declaratively.

For example, instead of exposing open() and close() methods on a Dialog component, pass an isOpen prop to it.

I get the idea and largely agree for reusable components - not having state (which would need to be manipulated imperatively via refs) hidden inside a component makes it easier to reason about and reuse. I'm wondering if this is applicable for concrete dialogs in an application also; if so I'm missing the understanding to use the right tools it seems.

For example: in my application, I have a file tree that shows a menu with options to create, rename, and delete files. Each option will show an appropriate dialog, and these dialogs are encapsulated in their own components. At the bottom of the component hierarchy is a reusable Dialog component:

FileTree
  FileMenu
  CreateFileDialog
    Dialog
  RenameFileDialog
    Dialog
  DeleteFileDialog
    Dialog

I can see two approaches here:

  • FileTree manages all three dialogs' visibility states, meaning no refs
  • the individual dialogs or even the underlying reusable dialog managing visibility state, using refs for showing dialogs when needed

In my concrete case Dialog is a third-party stateless component, but I could add a stateful wrapper for the second approach. And to me, that seems far superior: there is only a single instance of state handling code, encapsulated in the "correct" component. Having three instances of visibility state in FileTree means I need a way to distinguish these states (long names such as createFileDialogVisible or namespacing objects such as createFileDialog.visible).

(In practice there is additional state to be managed, e.g. the new file's name, but that is out of the scope of what the title asks. That said, I think it does impact the feasibility and clarity of putting all dialog state into FileTree.)

Given that I feel option two is so clearly superior, I can't imagine that the guideline is meant to encourage the first. Am I missing an alternative approach, or is this just something that is outside the scope of this guideline?


Appendix

Steps in my reasoning that I think are good candidates for being wrong:

  • the only alternative to using refs is pulling state into the parent component
  • managing state in the parent component makes the described code duplication/namespacing issues unavoidable
  • using refs will actually result in less code duplication/more ergonomic code

The following code sample shows a stateless dialog (Dlg), a stateful wrapper (Dialog) and a Consumer for that stateful dialog. In approach 1, FileTree corresponds to Dialog except that there would be multiple visibility states. In approach 2, FileTree corresponds to Consumer, again with multiple refs.

import * as React from 'react';

function Dlg({ visible, onClose }) {
  return visible ? (
    <div>
      dialog is shown
      <button onClick={onClose}>close</button>
    </div>
  ) : (
    <dvi>dialog is hidden</dvi>
  );
}

function Dialog(props, ref) {
  const [visible, setVisible] = React.useState<boolean>(false);

  const show = () => setVisible(true);
  const hide = () => setVisible(false);

  React.useImperativeHandle(ref, () => ({ show, hide }));

  return <Dlg visible={visible} onClose={hide} />;
}

Dialog = React.forwardRef(Dialog);

function Consumer() {
  const dialogRef = React.useRef(null);

  const showDialog = () => {
    // eslint-disable-next-line no-throw-literal
    if (dialogRef.current === null) throw 'ref is null';

    dialogRef.current.show();
  };

  return (
    <div>
      <div>
        <button onClick={showDialog}>open</button>
      </div>
      <Dialog ref={dialogRef} />
    </div>
  );
}
  • 1
    React is about managing state (isOpen), not so much about managing state transitions (open()). That is a design decision. A possibly reasonable design decision as it can be easier to keep state consistent that way (some transitions are illegal in some states, e.g. what happens when you open the dialog multiple times?). But it's also perfectly reasonable to disagree with that design decision. Many people find happiness outside of React dogma :) – amon Apr 20 at 16:01
0

You didn't clarify if multiple dialogs can be open at once.

If they can, then they can handle their own visibility state.

I'd they can't then I'd actually look at using a small state machine in FileTree, or hold it in React Context to prevent drilling.

That way you have a generic "setOpenDiog" piece of state which can be set to false, "create", "rename", "delete". Displaying the right one depending on the current selection.

| improve this answer | |
0

What has turned out to be wrong was the second step in my reasoning:

managing state in the parent component makes the described code duplication/namespacing issues unavoidable

When already using hooks, it is easy to extract this into its own hook and provide an API this way. The added benefit compared to refs is that the hook is not null before the first render, so there is no need for a null check to make a type checker happy. The realization to have is this: the Dialog component was purely behavior, it did not add any rendering beyond what Dlg already did; behavior is best abstracted in hooks.

This approach is for example used in material-ui-popup-state. Applied to the example in this question:

import * as React from 'react';

function Dlg({ visible, onClose }) { /* unchanged */ }

function useDialog() {
  const [visible, setVisible] = React.useState(false);

  const show = () => setVisible(true);
  const hide = () => setVisible(false);

  const bindDlg = () => ({
    visible,
    onClose: hide,
  });

  return { show, hide, bindDlg };
}

function Consumer() {
  const dialog = useDialog();

  return (
    <div>
      <div>
        <button onClick={dialog.show}>open</button>
      </div>
      <Dlg {...dialog.bindDlg()} />
    </div>
  );
}

material-ui-popup-state does it a little bit differently, in that it makes you write <Menu {...bindMenu(popupState)}> instead of <Menu {...popupState.bindMenu()}>. I see two reasons to do it that way:

  • bindMenu can be declared as a regular function, there is no question whether to memoize the function via useCallback.
  • besides bindMenu there is also bindPopover and others. by making it not part of the hook, the hook itself can remain more agnostic of the component onto which it attaches props.
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.