[RFC] Allowing private data on nodes, making parsing/encoding more flexible

Good morning.

We found ourselves looking for a way to store data on PM nodes, that are not attributes.

Nodes, as immutable value objects, hold their information in attrs, however as our nodes and node views became more complex, we wanted to persist more information.

We have developed a well-defined JSON document structure, which is used to validate nodes, marks and their respective attrs. There are cases, when we have run-time information that we don’t want to serialize, but we need to store with the node instance.

Example use case

For example, imagine an image node spec:

image: NodeSpec = {
  group: 'inline',
  inline: true,
  attrs: {
    src: { default: '' },
    alt: { default: null },
    title: { default: null }
  },
  draggable: true,
  parseDOM: [{
    tag: 'img[src]', getAttrs(dom: HTMLElement) {
      return {
        src: dom.getAttribute('src'),
        alt: dom.getAttribute('alt'),
        title: dom.getAttribute('title')
      };
    }
  }],
  toDOM(node: Node) { return ['img', node.attrs]; }
};

Let’s say, we want to add the source file information - file-name and file-mime-type. We could store them as properties on node and we would extend toDOM() this way:

image: NodeSpec = {
  // ...
  toDOM(node: Node) { 
    return [
      'img',
      { 
        ...node.attrs,
        'data-file-size' : node.fileSize,
        'data-file-name' : node.fileName
      }
    ];
  }
};

That takes care of encoding to DOM, but the opposite operation is not as trivial. Because parseDOM does not allow invokables, we must return a ParseRule structure which is then interpreted internally by DOMParser.

Solution 1 - Allow invokables for parseDOM, making parsing more flexible

We already use Node Views to full extent, and also used toDOM() which is allowed to return the final DOM element. We’ve noticed, however, that the same is not allowed for parseDOM:

parseDOM: ?[ParseRule] Associates DOM parser information with this node, which can be used by DOMParser.fromSchema to automatically derive a parser. The node field in the rules is implied (the name of this node will be filled in automatically). If you supply your own parser, you do not need to also specify parsing rules in your schema.

It means that the only way to implement custom parsing for a particular node/mark type is to subclass the whole DOMParser (and ParseContext, NodeContext, MarkContext to go with them).

Proposed architecture

I believe it’d be beneficial to enable parseDOM() to be a function, with the following signature:

type InvokableParseRule = (el: dom.Node, context: ParseContext) => Node | Mark | undefined;

interface NodeSpec {
  // ...
  parseDOM: ParseRule | InvokableParseRule
}

Here’s an example implementation:

image: NodeSpec = {
   parseDOM: (dom, parseContext) => {
    // short-circuit if it's not parseable here
    if (!dom.matches('img')) {
      return;
    }

    // let's construct the node as it would be normally by DOMParser
    const nodeType = parseContext.parser.schema.nodes.image;
    const node = nodeType.create({
      src: dom.getAttribute('src'),
      title dom.getAttribute('title'),
      alt: dom.getAttribute('alt') || ''
    });

    // handle aux properties that are not attrs
    if (dom.hasAttribute('data-file-name')) {
      node.fileName = dom.getAttribute('data-file-name');
    }

    if (dom.hasAttribute('data-file-size')) {
      node.fileSize = dom.getAttribute('data-file-size');
    }

    return node;
  },
}

I’ve created a prototype DOMParser subclass that allows invokable parseDOM: PM DOMParser allowing invokables for parseDOM · GitHub

This solution:

  • :+1: Is BC
  • :+1: If implemented by consumer correctly, will be as performant, or faster than, normal ParseRule (internally it also calls Element.matches())
  • :+1: Will implicitly be used for copy-paste.
  • :+1: Doesn’t require extra wiring, node-specific data stays with the node instance.
  • :small_red_triangle_down: Requires modification/sub-classing DOMParser.

Solution 2 - parseDOM.getAttrs(), a map, handlePaste and scanning.

Another workaround to the problem we’ve found is to:

  1. For parsing, have NodeSpec.parseDom[].getAttrs() method read the extra attributes:
  • For private properties, read and store them in a shared Map
  • For everything else, return them as attrs
  1. For pasting - implement EditorProps.handlePaste() on each Editor that uses this node.
  • Go through the Slice that has been created from clipboard data.
  • Find image nodes
  • Use the aforementioned Map to re-apply private properties on node instances.
  1. For modifying the document:
  • Every modification which might involve insert of image nodes will require another scan.
  • When constructing document, we need to run another pass to re-apply private properties.

This solution:

  • :+1: Doesn’t require sub-classing DOMParser.
  • :+1: is backwards compatible
  • :small_red_triangle_down: Breaks SOC because handlePaste() is on EditorProps, while NodeSpec is part of schema, which itself (and any of its nodes) could be shared across different editors.
  • :small_red_triangle_down: Is hard to test, because we can’t test the DOMParser directly (as we do for all other nodes)
  • :small_red_triangle_down: Less performant - requires scanning Slice for nodes on each paste.
  • :small_red_triangle_down: Requires injection/sharing of the Map in various places.
  • :small_red_triangle_down: Paste is prone to paste-related bugs.
  • :small_red_triangle_down: Might be hard to maintain because of multiple places where we mutate nodes.

cc @bradleyayers

Another angle to consider is decoupling the ProseMirror schema from the data-at-rest (e.g. stored in a database) schema. Specifically this would mean not using Node.toJSON() verbatim to produce the data-at-rest value.

ProseMirror already has a way to associate metadata with nodes — attrs. That mechanism sounds like it perfectly suites this scenario, except that an additional constraint is being imposed that the ProseMirror schema must be equivalent to the data-at-rest schema.

This is already the case with some of your data, e.g. the mention_query mark.

Something else to consider is the collaborative editing story. If you want collaborators to receive the run-time information, it needs to be serialised and sent over the wire. If you use attrs you’ll have a great time, but if you go expando properties you probably won’t.

@bradleyayers the trouble with this approach is the decoupling between node instances and the extra data.

There’s two things that I noticed though:

  1. It makes sense to store extra data outside of the lifecycle of PM if you’re only concerned with encoding/parsing – What you’re suggesting applies to the data-at-rest when using serialisers but won’t solve a case, where you want to use the data in real-time to make decisions i.e. in node views, paste handlers, interaction logic etc. In such case, the data lives somewhere outside and is not easy to be coupled back to i.e. a transaction Step’s items.
  2. When both datasets (let’s call them attrs and props) are decoupled, you need multiple convergence mechanisms to put them back together when needed - I’ve listed that as a con to the second solution. Whenever working with those nodes, even in simple case of just needing this information when serialising (encoding), you need to produce a joined set, resolving the initial relations between data and node instances. Because nodes are immutable, come in and out, it might be hard to track.

There might be another solution:

Solution 3 - make private data an API

In order to prevent polluting the node instance, we could have an API for custom data.

NodeView
    // ...
    attrs: ?Object<AttributeSpec>,
    data: ?Object,
```
`node.data` would be carried over in transactions (flat-copied), can be populated with anything the consumer wants, would be otherwise ignored by default parsers, encoders and toJSON() (unless i.e. `toDOM()` explicitly uses it).

We'd need to couple it with "Solution 1" parseDOM invokables for completeness.

Could you go back and explain the problem in more detail? I can’t really figure out from your solutions what they are intended to solve.

I want to store private data on nodes, which then I can use in other classes (i.e. encoders/parser that we have, nodeviews etc.). This data is not supposed to be serialised when building document with toJSON().

Currently that’s not easy to archive because of the architecture. I’m proposing small changes that would allow this.

I think @bradleyayers’ suggestion addresses this pretty well. It should also be pretty easy to write your own JSON-serializing function if you want to include custom behavior.