React Functional component unmount

Question

I want to rewrite this component https://github.com/josdejong/jsoneditor/blob/master/examples/react_advanced_demo/src/JSONEditorReact.js to a functional component. I made this:

const Editor = ({json, mode, onChange}) => {
  const elRef = useRef(null);

  useEffect(() => {
    const options = {
      onChangeText: onChange,
      onChangeJson: onChange,
    };

    const jsonEditor = new JSONEditor(elRef.current, options);
    if (jsonEditor) {
      if (json) {
        jsonEditor.set(json);
      }

      if (mode) {
        jsonEditor.setMode(mode);
      }
    }

    return () => {
      if (jsonEditor) {
        jsonEditor.destroy();
      }
    }
  },[json,mode]);
  
  return (
    <div
      className={styles.jsoneditor_react_container}
      ref={elRef}
    />
  )
}

export default Editor;

My question is next: Is it correct to unmount the component jsonEditor.destroy(); inside that useEffect with [json, mode] dependancies or i should create another useEffect with empty dependancy array to get the same behaviour as in class component?

Answer 1

What you're doing is fine provided that destroying and rebuilding the JSONEditor doesn't take discernible time or cause a visual twitch that's apparent to the user. The editor will be cleaned up when the component is unmounted.

But, since it seems to allow you to change mode and JSON on an existing instance (based on the API), I think I'd do that rather than completely destroying it and rebuilding it when those change:

const Editor = ({json, mode, onChange}) => {
  const elRef = useRef(null);
  const edRef = useRef(null);

  useEffect(() => {
    // On mount
    const options = {
      onChangeText: onChange,
      onChangeJson: onChange,
    };

    const jsonEditor = edRef.current = new JSONEditor(elRef.current, options);
    if (json) {                 //
      jsonEditor.set(json);     // I don't think you actually need these here,
    }                           // I *think* `useEffect` callbacks are run in
    if (mode) {                 // order so the below will do it. But...
      jsonEditor.setMode(mode); //
    }                           //

    return () => {
      // No `if`, you *know* it exists
      jsonEditor.destroy();
      edRef.current = null;
    }
  }, []);

  useEffect(() => {
    const jsonEditor = edRef.current;
    if (!jsonEditor) { // I don't think this can happen
      return;
    }
    if (json) {
      jsonEditor.set(json);
    }
    if (mode) {
      jsonEditor.setMode(mode);
    }
  }, [json, mode]);
  
  return (
    <div
      className={styles.jsoneditor_react_container}
      ref={elRef}
    />
  );
};

export default Editor;

But that's more complicated, so if what you're doing works well and provides the user experience you want, you might want to leave it alone.