lexical icon indicating copy to clipboard operation
lexical copied to clipboard

Bug: registerUpdateListener fires the function once more when the editor mounts or unmounts

Open YugyeomKim opened this issue 10 months ago • 13 comments

Lexical version: 0.24.0

Steps To Reproduce

Add an registerUpdateListener of an editor object from useLexicalComposerContext in a useEffect hook.

Code example:

import type { SerializedEditorState } from 'lexical';

import { useLexicalComposerContext } from '@lexical/react/LexicalComposerContext';
import debounce from 'lodash.debounce';
import { useCallback, useEffect } from 'react';

type UpdatePluginProps = {
  onChange: (editorState: null | SerializedEditorState) => void;
};

export default function UpdatePlugin({ onChange }: UpdatePluginProps) {
  const [lexicalEditor] = useLexicalComposerContext();

  // eslint-disable-next-line react-hooks/exhaustive-deps
  const saveData = useCallback(
    debounce(() => {
            // Do something here like saving the contents to db
      });
    }, 1000),
    [lexicalEditor],
  );

  useEffect(() => {
    const unregisterListener = lexicalEditor.registerUpdateListener(saveData);

    return () => {
      unregisterListener();
    };
  }, [lexicalEditor, saveData]);

  return null;
}

The current behavior

After updating lexical version from 0.18.0 to 0.24.0, registerUpdateListener fires the function passed as a parameter whenever the editor mounts or unmounts. This execution of the function happens before the cleaner of the useEffect runs, making it hard to handle the execution properly.

The expected behavior

  • Option 1: The listener function doesn't run whenever the editor mounts or unmounts.
  • Option 2: There is an option for registerUpdateListener to disable this functionality

Impact of fix

We are using an index-based approach to perform CRUD operations on an editor (in reality the parent of the editor). When an editor is removed from the array, the index shifts and points to a different editor. Since we are using lodash.debounce inside the update listener function, the debounced update from the removed editor ends up overwriting the data of a newly added editor in the DOM. We are unable to selectively cancel the debounced function when the editor unmounts.

YugyeomKim avatar Feb 18 '25 07:02 YugyeomKim

Why don’t you include a reference to the editor in your callback and ignore it if it doesn’t match the editor in your array? Using React’s key property correctly might also fix this for you

etrepum avatar Feb 18 '25 14:02 etrepum

You can also have a ref that tracks if the editor is unmounted and return from that debounced function body when that has happened.

etrepum avatar Feb 18 '25 14:02 etrepum

@etrepum

Thanks for the suggestions! Unfortunately, they don’t quite fit our case.

We want to keep the debounced function whose timer has been started when the editor is unmounted, so that any changes made right before the editor’s removed get saved to our DB. It seems like your approach would prevent that from happening.

That said, we’re still having issues with the debounced function running as the editor is being unmounted—it’s causing problems in our app.

YugyeomKim avatar Feb 18 '25 15:02 YugyeomKim

Using React’s key property correctly might also fix this for you

I've tried assigning a unique key for each editor, but it didn't work.

YugyeomKim avatar Feb 18 '25 15:02 YugyeomKim

Certainly passing the key through to your callback so you can verify that the update came from the editor you expected to should work.

That said, if you can provide a runnable reproduction of this issue (e.g on stackblitz or codesandbox) I can look into it further.

etrepum avatar Feb 18 '25 15:02 etrepum

Having the same problem, and I individuated the regression (or breaking change, at least) in v0.23.0. The difference, as above stated, is that there is an extra fire when the component unmounts.

export default function OnChangePlugin({ handleChange }) {
  const [editor] = useLexicalComposerContext();

  useEffect(
    () =>
      editor.registerUpdateListener(({ editorState }) => {
        console.log('🔥 ~ editorState:', editorState);

        editorState.read(() => {
          handleChange(editorState, editor);
        });
      }),
    [editor, handleChange],
  );

  return null;
}

the console log won't log in v0.22.0 if component was unmounted, but it is logging now after v0.23.0 and caused some bugs.

buondevid avatar Feb 25 '25 22:02 buondevid

I still haven't seen a complete example that reproduces this issue. If you post a complete runnable example that reproduces the issue, it will be investigated.

etrepum avatar Feb 25 '25 23:02 etrepum

I haven't seen any evidence that any extra updates happen on unmount but there are certainly ways that you can get an extra update when a new editor is mounted, such as when using a plugin that will manipulate the selection (e.g. AutoFocusPlugin). This is why the OnChangePlugin has an ignoreSelectionChange option to ignore updates where only the selection has changed. In the incomplete code snippets that you both have pasted, there doesn't appear to be any logic that ignores those sorts of selection-only updates.

etrepum avatar Feb 25 '25 23:02 etrepum

Thanks for looking into this! I will post a repro when I get time, but one thing is certain: the editor update doesn't happen with version 0.22, and does happen with version 0.23, so API or not something changed in that bump.

buondevid avatar Feb 26 '25 05:02 buondevid

Sometimes these things happen because the previous behavior was incorrect.

etrepum avatar Feb 26 '25 06:02 etrepum

However, the previous behavior shouldn’t be considered incorrect, as any editor-state updates would have already been processed before the editor was unmounted. There doesn’t seem to be any benefit to triggering it again upon unmount.

YugyeomKim avatar Feb 26 '25 06:02 YugyeomKim

So far I have not seen any evidence that any update events happen on unmount. I tried to reproduce it by taking the example app and providing a button to toggle the rendering of the editor and events are only fired when it mounts or when it is updated, never when it is unmounted. I can not do any further investigation until someone provides code that I can run that demonstrates the issue.

etrepum avatar Feb 26 '25 06:02 etrepum

So far I have not seen any evidence that any update events happen on unmount. I tried to reproduce it by taking the example app and providing a button to toggle the rendering of the editor and events are only fired when it mounts or when it is updated, never when it is unmounted. I can not do any further investigation until someone provides code that I can run that demonstrates the issue.

Totally get it, and to be fair when trying to make a repro online I also don't notice this behavior, so it must connected to something specific we do or use in our implementations. Will ping you back if I manage to reproduce in a trivial app. Thank you for the effort for now 🙏 .

buondevid avatar Feb 26 '25 09:02 buondevid

Please re-open or create a new issue when you have a full example that reproduces this issue.

etrepum avatar Mar 13 '25 05:03 etrepum