modularize yDocToProsemirrorJSON to Y.XmlText and Y.XmlElement

[BrianHung]

Motivation for this PR is to separate out Y.XmlFragment from Y.XmlElement in the scenario of using a custom top-level document with type, attrs, and marks defined.

Related:
https://discuss.yjs.dev/t/y-prosemirror-ydoctoprosemirrorjson-root-node-type-is-always-doc/642
#48

sync-plugin still needs to be refactored to allow for XmlElement to be a top-level document, and #63 is related because the new YXmlElementToProsemirrorJSON method assumes node-level marks are stored as an array in type.getAttribute('marks). Also in #63, node-level marks are serialized using toJSON() which is pretty convenient for this PR as well.

[dmonad]

Hi @BrianHung,

thanks for working on this. I will give this PR another review if you leave the existing API functional. Unless there is a good reason, we can't change the arguments or the names of the functions without breaking existing code.

[dmonad]

Please don't use for-in loops for arrays as they fail to transpile in old typescript versions. I deliberately only use normal for loops.

[dmonad]

The .flat() method is a fairly recent addition to EcmaScript. Please use import { flatten } from 'lib0/array' instead.

[SamDuvall]

@BrianHung, @dmonad, I don't think yDocToProsemirrorJSON is the correct signature to convert a Y.XmlFragment to JSON. In my mind, a Y.XmlFragment is an array of Nodes, not a Node with type 'doc'.

See my PR here: #53

[BrianHung]

I don't think yDocToProsemirrorJSON is the correct signature to convert a Y.XmlFragment to JSON. In my mind, a Y.XmlFragment is an array of Nodes, not a Node with type 'doc'.

@SamDuvall I agree with both of your points about separating yDoc from Y.XmlFragment. YXmlFragmentToProsemirrorJSON is pretty simple with the newly modularized functions -- just needed that serialize function to be outside of it -- so I've added that as well.

[dmonad]

Thanks to @BrianHung and @SamDuvall we now have two competing implementations 🤔

#53
#64

After reviewing this I think you are right that we should change the signature. I would be fine with making another major release for this feature. Could you both please communicate and come up with a solution that works for both of you. I'll merge the version that you both reviewed and agree with. Thanks!

[SamDuvall]

This PR has everything I need for converting Y.Xml* to JSON. I am happy to abandon #53 and then create a new PR that focuses on JSON to X.XmlFragment.

[BrianHung]

we should change the signature

To clarify, this would mean in essence deprecating yDocToProsemirrorJSON in favor of the other two utility methods YXmlFragmentToProsemirrorJSON and YXmlElementToProsemirrorJSON?

Because if we change the signature of yDocToProsemirrorJSON to return "an array of Nodes" as Sam suggested, that would be equivalent to YXmlFragmentToProsemirrorJSON.

In my view, deprecating yDocToProsemirrorJSON would be good because it'd force the end user to explicitly work with YXMLFragment or YXMLElement instead of that being abstracted behind Y.Doc and a field.

Those using the new API but want the old abstraction of yDocToProsemirrorJSON can easily migrate by:

// Previously
let json = yDocToProsemirrorJSON(ydoc)

// Now
let xmlFragment = ydoc.getXmlFragment('prosemirror')
let json = {type: 'doc', content: YXmlFragmentToProsemirrorJSON(xmlFragment)}

[SamDuvall]

If we're doing a major version bump, then I agree with @BrianHung's proposal to just delete yDocToProsemirrorJSON, so long as there is a clear explanation in the release notes.

@dmonad, if we did a major version bump, would you be okay with taking #34 as-is or would you still like a PR that is backwards-compatible? I'm coming across some issues with annotations/comment/suggestions that would benefit greatly from marks of the same type (e.g. comment) being allowed.

[flaviouk]

This would be a great addition!

I also need to be able to set prosemirror content json to a YFragment, I believe updateYFragment does this, but I'm curious if we should create a separate PR for that:
#53

[EricHasegawa]

Any updates here? Having the same issue needing overlapping nodes with different attributes