DecorationSets and Remapping broken with y-sync plugin

1

Hello

It appears that DecorationSet remapping is broken when using y-sync.

decorationSet.map(transaction.mapping, transaction.doc);

With YJS we have to create a new decoration set every time - This is expensive… See below for the example… Any thoughts, ideas or potential solutions would be great.

import { Plugin, PluginKey, EditorState, Transaction } from 'prosemirror-state';
import { DecorationSet, Decoration } from 'prosemirror-view';

// This doesnt work:
export const efficient = new Plugin({
  key: new PluginKey('sample'),
  state: {
    init(_: unknown, editorState: EditorState): any {
      return DecorationSet.create(editorState.doc, [
        Decoration.inline(10, 20, {
          style: 'color: red;',
        }),
      ]);
    },
    apply(transaction: Transaction, decorationSet: DecorationSet): any {
      return decorationSet.map(transaction.mapping, transaction.doc);
    },
  },
  props: {
    decorations(state): any {
      return this.getState(state);
    },
  },
});

// Only a new DecorationSet each time works.
// In our case we are creating a new decorationSet on every key press - This is expensive.
export const inefficient = new Plugin({
  key: new PluginKey('sample'),
  state: {
    init(_: unknown, editorState: EditorState): any {
      return DecorationSet.create(editorState.doc, [
        Decoration.inline(10, 20, {
          style: 'color: red;',
        }),
      ]);
    },
    apply(transaction: Transaction): any {
      return DecorationSet.create(transaction.doc, [
        Decoration.inline(10, 20, {
          style: 'color: red;',
        }),
      ]);
    },
  },
  props: {
    decorations(state): any {
      return this.getState(state);
    },
  },
});
2

Huh haven’t even considered that. Hmm yeah yjs transactions from other origins than yourself always replace the whole doc. Well the initial loading of yDoc does that too. From looking at it, there doesn’t appear to be a straightforward solution to this. I mean the cursor plugin doesn’t map the decoration set either, just creates a new one on changeOrigin / awarenessUpdated transactions

To actually just remap it seems to require some intricate hacking.

You can find the range where the states have changed at least with:

    apply(transaction: Transaction, decorationSet: DecorationSet): any {
      const diffEnd = oldState.doc.content.findDiffEnd(newState.doc.content)
      if (diffEnd) {
        console.log('diffStart', oldState.doc.content.findDiffStart(newState.doc.content))
        console.log('diffEnd', diffEnd)
      }
      return decorationSet.map(transaction.mapping, transaction.doc);
    },

It seems findDiffEnd will always find the changed part at least. Eg if you inserted a 10 size slice at 5, it outputs: { a: 5, b: 15 }. And if you delete content say between 10-20 it returns { a: 20, b: 10 }. Using that, I guess you could generate a mapping that shifts the decorations one way or the other.

Won’t write a PoC but you can try that one out.

3

When you say “This is expensive”, do you have any quantitative measurement of how expensive?

We noticed this pattern as well (when isChangeOrigin is true, all decorations are recreated) and thought it would be expensive, but in practice it appears Prosemirror is smart enough to take the new decoration set and only actually redraw to the DOM when necessary.

4

Hey, thanks for the response! It is interesting. When I say inefficient. It’s more about preparing the decorationSet rather than rendering it in this case.

In some of our documents, we have a lot of nodes and marks and we have to loop them to find all the decoration positions (There can be a lot…). In an ideal world, we want to loop the document and find those decoration positions as little as possible.

With ySync not supporting remapping our plugins are having to find all of those decorations again and again (on every ySync)… and in doing so… they are doing much more work than they were before…

In short if we think of a decorationSet as a cache. ySync currently invalidates the cache.

An example… now we are having to change plugins like this…

state: {
    init(_: unknown, editorState: EditorState): any {
      return getAllDecorations(editorState.doc);
    },
    apply(transaction: Transaction, decorationSet: DecorationSet): any {
      // if yjs find all positions again...
      const yjsSync = transaction.getMeta('y-sync$');
      if (yjsSync) return getAllDecorations(transaction.doc);
      
      // Only update parts of decoration cache based on transaction
      // This can remap and only update decorations that are within the transaction steps selection ranges.
      return updateAffectedDecorations(transaction, decorationSet); 
    },
  },

For context… We are moving away from operational transforms using prosemirror-collab and we discovered this as part of the switch over to YJS. It does change the behaviour of Prosemirror plugins I’m interested to explore @TeemuKoivisto example above to see if there’s a way to make an adaptor of some kind. In my mind, it shouldn’t matter what collab algo you are using… plugins should always work the same way.

1 Like
5

It’s not just the inefficiency, this catch also means that you can’t use yjs with many prosemirror plugins that rely on standard decoration behavior.

FWIW I brought this up with Kevin previously and he said that the behavior wouldn’t be fixed at the time:

2 Likes