-
-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhance Y.XMLFragment to/from JSON serialization #53
base: master
Are you sure you want to change the base?
Conversation
I've had a slight change of heart from my initial comment. I really only needed However, I do think the signature for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Please just clarify this one thing.
@@ -1,5 +1,5 @@ | |||
export * from './plugins/cursor-plugin.js' | |||
export { ySyncPlugin, isVisible, getRelativeSelection, ProsemirrorBinding } from './plugins/sync-plugin.js' | |||
export { ySyncPlugin, isVisible, getRelativeSelection, updateYFragment, ProsemirrorBinding } from './plugins/sync-plugin.js' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need updateYFragment
? I don't think this should be exported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, what I really want is prosemirrorJSONToYFragment
instead of prosemirrorJSONToYDoc
. Should I make a separate PR or amend this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering, would prosemirrorJSONToYFragment
also allow for the conversion of json to Y.XMLElement
?
I'm looking through y-prosemirror
looking for blockers preventing people from using Y.XMLElement
as the type (in the case of a custom top document), and noticing prosemirrorJSONToYDoc
hardcodes the Y.XMLFragment
with ydoc.get(xmlFragment, Y.XmlFragment)
.
Right now, one can't make a prosemirrorJSONToYXmlElement
because updateYFragment
isn't exported as you pointed out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateYFragment
exported with a slightly cleaner interface would be incredibly useful. Perhaps something with the signature of: applyProsemirrorToYFragment(ydoc, prosemirrorDoc)
If the
y-prosemirror
utility functions were called from aY.XMLElement
perspective, instead of aY.Doc
with a xmlFragment key perspective, developers could re-use the utility functions to perform Y.XMLElement updates on the server. This would give developers another tool to build documents on a server before they are served to clients.