Skip to content

Create a new shared xml model for real time collaboration #89

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

Closed
wants to merge 11 commits into from

Conversation

hbcarlos
Copy link
Contributor

@hbcarlos hbcarlos commented May 11, 2021

As discussed in jupyterlab/jupyterlab#10227, this PR is an example of creating new document models for real-time collaboration in JupyterLab.

TODO:

  • Update dependencies
  • Create a new DocumentModel
  • Create a new xml ISharedModel by extending from YDocument
  • Observe shared model in DrawIOWidget
  • Create types for events (docs)
  • Parse events

@hbcarlos hbcarlos marked this pull request as draft May 14, 2021 09:20
@jtpio
Copy link
Member

jtpio commented May 18, 2021

Thanks for starting this!

@hbcarlos
Copy link
Contributor Author

hbcarlos commented May 18, 2021

I'm having an error with yjs coming from:

this._root.insert(0, [new Y.XmlElement('cell')]);

Looks like there are two instances, one coming from JupyterLab and another from the extension.

9795.87a9ff4f3c3919e683d2.js?v=87a9ff4f3c3919e683d2:1 Uncaught Error: Unexpected content type in insert operation
    at 9795.87a9ff4f3c3919e683d2.js?v=87a9ff4f3c3919e683d2:1
    at Array.forEach (<anonymous>)
    at Vs (9795.87a9ff4f3c3919e683d2.js?v=87a9ff4f3c3919e683d2:1)
    at Ps (9795.87a9ff4f3c3919e683d2.js?v=87a9ff4f3c3919e683d2:1)
    at 9795.87a9ff4f3c3919e683d2.js?v=87a9ff4f3c3919e683d2:1
    at Yn (9795.87a9ff4f3c3919e683d2.js?v=87a9ff4f3c3919e683d2:1)
    at mr.insert (9795.87a9ff4f3c3919e683d2.js?v=87a9ff4f3c3919e683d2:1)
    at new Q (646.3107e1e3e0432c8636f0.js?v=3107e1e3e0432c8636f0:1)
    at Function.create (646.3107e1e3e0432c8636f0.js?v=3107e1e3e0432c8636f0:1)
    at new z (646.3107e1e3e0432c8636f0.js?v=3107e1e3e0432c8636f0:1)

This line returns false, being ydoc an instance coming from JupyterLab and Y.XmlFragment a class imported from Yjs in the extension.

this.ydoc.getXmlFragment('root') instanceof Y.XmlFragment

console.debug(this._root instanceof Y.XmlFragment);

cc @jtpio @dmonad

@bollwyvl
Copy link
Contributor

bollwyvl commented Jun 2, 2021

Cool stuff here!

3.1.0a11 just dropped on pip which ships with the singleton yjs which should help unblock things.

For ipydrawio, which supports both top-level mxGraphModel and mxfile -> diagram+ -> mxgraphmodel, as well as the svg-backed format (which is just an icky <svg content="&lt;mxfile..., this will get kinda messy. While direct string-banging works for XML... I do wonder if there's some way we might be able to affect something like it with a more structured approach (e.g. nunjucks, xslt) which might handle more of the weird corner cases, and scale up to something like the svg namespace?

@hbcarlos
Copy link
Contributor Author

hbcarlos commented Jun 3, 2021

Hey, thanks, @bollwyvl.

I'm sorry, but I do not understand what you mean.

@bollwyvl
Copy link
Contributor

bollwyvl commented Jun 3, 2021

In getSource and setSource, the approach knows exactly about the "schema" (if only!) of the mxGraphModel. Up on diagrams.net, they generate xml files that wrap multiple mxGraphModel in a few layers to provide multiple pages, etc. I'd like to be able to adapt this work over there, but the closer we can bring the approaches together, the better probably.

@hbcarlos
Copy link
Contributor Author

hbcarlos commented Jun 3, 2021

Ooh, got it. And what's your suggestion for a more generic model?

@bollwyvl
Copy link
Contributor

bollwyvl commented Jun 3, 2021

Right, XML isn't all the rage, but there are tons of important formats in it. If the ISharedXMLFile was generic over some schema, it might eventually become something that could live up in core... we don't need everybody doing this slightly differently.

Another idea i've had a few times is giving up, and finding a compatible pair of js python and XML-to-json round-trip convertors. As one of the models we support on the fork is stuffing things into a notebook's metadata, having it be JSON-native would be pretty nice... until it pulls shenanigans like base64 encoding xml-in-xml.

@hbcarlos
Copy link
Contributor Author

hbcarlos commented Jun 3, 2021

Maybe we can make it more generic using Y.Maps instead of Y.XxmlFragments and Y.XmlElements.
We could create a schema like:

"node": {
    "attributes": { ... },
    "child_1": {
        "attributes": { ... },
        "child_1_1": {
            "attributes": { ... },
            "text": "whatever"
        },
        ...
    },
    "child_2": {
        "attributes": { ... },
        "child_2_1": [
            "child_2_1",
            "child_2_1",
            "child_2_1",
            ...
        ],
        ...
    },
    ...
}

Where an element could be either object, list or string. Identifying the attributes as "attributes" and a text child as "text"

@hbcarlos
Copy link
Contributor Author

hbcarlos commented Jun 3, 2021

Instead of attributes and text we should use maybe #attributes and #text.

@hbcarlos hbcarlos mentioned this pull request Jun 28, 2021
6 tasks
@hbcarlos
Copy link
Contributor Author

This PR continues in #99

@hbcarlos hbcarlos closed this Jun 28, 2021
bollwyvl added a commit to bollwyvl/jupyterlab-drawio that referenced this pull request Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants