yjs / y-prosemirror

ProseMirror editor binding for Yjs
https://demos.yjs.dev/prosemirror/prosemirror.html
MIT License
335 stars 116 forks source link

Uncaught TypeError: Cannot read properties of null (reading 'matchesNode') #70

Closed hoebbelsB closed 2 years ago

hoebbelsB commented 2 years ago

Please save me some time and use the following template. In 90% of all issues I can't reproduce the problem because I don't know what exactly you are doing, in which environment, or which y-* version is responsible. Just use the following template even if you think the problem is obvious.

Checklist

Describe the bug The syncPlugin will run into an error if the editor get's destroyed. It looks like a call to tiptap#unregisterPlugin causes every other plugin view to update. sync-plugin.js will then schedule a new _forceRenderer() cycle:

// sync-plugin.js

setTimeout(() => {
  binding._forceRerender()
  view.dispatch(view.state.tr.setMeta(ySyncPluginKey, { binding }))
}, 0)

When the editor gets destroyed in the meantime, the scheduled function will cause the following error:

index.es.js:4914 Uncaught TypeError: Cannot read properties of null (reading 'matchesNode')
    at EditorView.updateStateInner (index.es.js:4914)
    at EditorView.updateState (index.es.js:4885)
    at Editor.dispatchTransaction (tiptap-core.esm.js:3102)
    at EditorView.dispatch (index.es.js:5162)
    at sync-plugin.js:282
    at ProsemirrorBinding.mux (mutex.js:35)
    at ProsemirrorBinding._forceRerender (sync-plugin.js:278)
    at sync-plugin.js:134

just noticed,... the same is true for setMeta which calls eventloop.timeout:

index.es.js:4914 Uncaught TypeError: Cannot read properties of null (reading 'matchesNode')
    at EditorView.updateStateInner (index.es.js:4914)
    at EditorView.updateState (index.es.js:4885)
    at Editor.dispatchTransaction (tiptap-core.esm.js:3102)
    at EditorView.dispatch (index.es.js:5162)
    at lib.js:30
    at Map.forEach (<anonymous>)
    at updateMetas (lib.js:25)

To Reproduce Steps to reproduce the behavior:

Expected behavior setTimeout handler should get cleared on destroy.

Environment Information

Additional Information

I currently monkeypatch the plugin:

const fragment = this.options.fragment
                         ? this.options.fragment
                         : this.options.document.getXmlFragment(this.options.field);
const syncPlugin: Plugin = ySyncPlugin(fragment);
let changedInitialContent = false;
let handler: number;
syncPlugin.spec.view = view => {
    const binding = new ProsemirrorBinding(fragment, view);
    // Make sure this is called in a separate context
    if (handler) {
        clearTimeout(handler);
    }
    handler = setTimeout(() => {
        binding._forceRerender();
        view.dispatch(view.state.tr.setMeta(ySyncPluginKey, { binding }));
    }, 0) as unknown as number;
    return {
        update: () => {
            const pluginState = syncPlugin.getState(view.state);
            if (pluginState.snapshot == null && pluginState.prevSnapshot == null) {
                if (changedInitialContent || view.state.doc.content.findDiffStart(view.state.doc.type.createAndFill().content) !== null) {
                    changedInitialContent = true;
                    binding._prosemirrorChanged(view.state.doc);
                }
            }
        },
        destroy: () => {
            clearTimeout(handler);
            binding.destroy();
        }
    };
};

If I got time, I will prepare a PR for it right away :)

dmonad commented 2 years ago

Fixed this in y-prosemirror@10.0.16