Can I get away with not using useEffect in this instance?-CodePudding

I'm still learning React and the proper way to do things. In this React project I am using the TinyMCE editor: https://www.tiny.cloud/docs/tinymce/6/react-cloud/

I want to display a warning message about unsaved changes once the editor has been made "dirty" that is that the user has modified the default content you get on reload.

The component that contains the editor has the following code:

<Editor
    apiKey='asdf'
    onInit={(_evt, editor) => editorRef.current = editor}
    onDirty={(_evt, _editor) => setEditorWasModified(true)}
    ...
</Editor>

Finally I have just put the code directly into the React component itself where it overwrites the window.onbeforeunload event:

const [editorWasModfied, setEditorWasModified] = useState(false);
const editorRef = useRef<TinyMCEEditor | null>(null);

window.onbeforeunload = () => {
    if (editorWasModfied) {
        return "You have unsaved changes. Are you sure you want to leave?";
    }
};

This code seems to run and work well.

So is my question here is this the correct approach to do this? Or do I need to use an useEffect hook or something similar? If I understand useEffect correctly, it runs on every render which is not something I want. I know there is a second argument, but since it refers to the editorWasModified variable, I get a warning that the dependency array should contain it, which is false since I only want the useEffect once to update the window.onbeforeunload event.

CodePudding user response：

What you have can work, though I'd consider it to be inelegant - every time there's a re-render, you're attaching a new beforeunload listener (and removing the previous one), no matter whether it's necessary or not. You're also not removing the listener when the component dismounts (which would be something to keep in mind if the page this is on has could exist without this component being mounted). After this component is unmounted, it'd be best for the listener with the editorWasModfied check to be removed as well.

If I understand useEffect correctly, it runs on every render which is not something I want.

Well, you're attaching it on every render currently anyway.

I know there is a second argument, but since it refers to the editorWasModified variable, I get a warning that the dependency array should contain it, which is false since I only want the useEffect once to update the window.onbeforeunload event.

The usual way to fix this lint issue is to attach a new listener whenever the state values it references changes. Using useEffect will also let you remove the listener when the component unmounts.

useEffect(() => {
  const handler = () => {
    if (editorWasModfied) { // perhaps fix typo: editorWasModfied -> editorWasModified
      return "You have unsaved changes. Are you sure you want to leave?";
    }
  };
  window.addEventListener('beforeunload', handler);
  return () => window.removeEventListener('beforeunload', handler);
}, [editorWasModfied]);

CodePudding user response：

You are correct that useEffect runs on every render (if no dependency array was specified), but so does your code without useEffect.

Here are the three different useEffect "settings":

Runs on every render:

useEffect(() => {

})

Run once:

useEffect(() => {

}, [])

Runs everytime editorWasModfied changes:

useEffect(() => {

}, [editorWasModfied])

Sometimes we only want the useEffect to run once, meaning we don't want any dependencies in our dependency array (second argument), but the linter complains about it.

WARNING Don't use the following as an easy escape from the linter warnings. You should always try to write code following linter rules!

With that said, to suppress this linter warning, you can use // eslint-disable-next-line, but only do this if you are absolutely sure that what you are doing is correct (suppressing the linter is an edge case and is not recommended unless absolutely necessary):

useEffect(() => {

  // eslint-disable-next-line
}, [])