De/serializing undo/redo stack

[Alexander Nestorov]

Hi!

I have been reading the CommandManager code in order to check how feasible it is to implement a de/serialization methods, so that I can "save" the undo/redo stack along with my model.

I see that the "undoStack" and the "redoStack" arrays are holding the data I'm interested in.

I also have been doing some tests in order to check if at any point / operation any of the objects stored in these 2 arrays contain any references to "memory" (instances of objects that can't be serialized), and I haven't seen any.

If I'm correct, de/serializing the CommandManager should be "as simple as" calling JSON.stringify() on both the "undoStack" and the "redoStack" (and JSON.parse() for the inverse operation).

Is that correct or am I missing something?

If this is the case, maybe this feature could be added to rappid?

Regards!

[Roman Bruckner]

Hi Alex,

This feature has been already requested. And it makes total sense to add it to Rappid.

The only concern I have at the moment are the `options` we store along with the attributes delta. If you add a model to a collection (a cell to graph) the Backbone sends extra info containing cell instances via options (you can test when you drop an element from the stencil).

Perhaps we can just filter it before storing. We have to take a closer look at this.

Best,

Roman

--

---
You received this message because you are subscribed to the Google Groups "JointJS" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jointjs+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jointjs/12dca04c-78c5-434c-948c-b28a2538a8b4n%40googlegroups.com.

--

Roman Bruckner, CTO

client IO

http://client.io

http://jointjs.com

+420 776 504 406

[Alexander Nestorov]

Hi Roman!!

Ah, indeed, I see what you mean. Once a new cell gets added, the undoStack has the following structure:

undoStack: [

    [

        {

            action: "add",

            batch: true,

            options: {

                add: true,

                changes: [ <-- newly added cell --> ]

            }

        }

    ]

]

After inspecting the cell, I think the cell could be re-created at runtime after deserializing the CommandManager.

It contains the following attributes that are primitives and / or can be serialized without problems:

* attributes

* changed

It also has the following attributes that we have at runtime:

* collection

* graph

An instance of CommandManager is linked to an instance of a model, so we can infer both of these attributes, which

means that we can re-created the object at runtime and then "attach" to it these two attributes.

The serialization (of the undoStack) should be as simple as:

const serialized = JSON.stringify(cm.undoStack);

This will partially serialize the stuff inside the "options". Of course we will lose the references of the collection and the

graph, but we can easily fix this later, when the data gets deserialized.

Then the deserialization operation of that data could be something like:

const deserialized = JSON.parse(serialized);

cm.undoStack = deserialized; // "cm" is an instance of CommandManager

deserialized.forEach(cell => {

    if(cell.options.add) { // We're checking only the "added" field. We should check "merged" and "removed" too

        const tmp = cell.options.changes.added[0]; // We're assuming only 1 cell was added. We should iterate over the entire array

        tmp.collection = cm.graph.attributes.cells; // is there a better way of getting the collection? This feels kind of hacky

        tmp.graph = cm.graph;

    }

});

The "contract" that the user should accept is that he/she must serialize the model and the commandManager at the same time

(read as in "first serialize the model, then serialize the CM without doing any changes to the model"). He/she must also deserialize

the model, then create a new CM with that model, then deserialize the CM data.

If a CM data is deserialized into a different model, behavior will be undefined as there is no way to predict what might happen.

How does that sound, Roman?

Regards!

[Roman Bruckner]

Hi Alex,

Well, I really hope this won't be necessary.

Backbone adds this structure (added, merged, removed)  to the options object just for the purpose of the `update` event.

https://backbonejs.org/docs/backbone.html#section-122

So I believe we can just prevent storing this `changed` option. When the user hits undo/redo the Backbone will put this structure back to the options object.

Best,

Roman

To view this discussion on the web visit https://groups.google.com/d/msgid/jointjs/9d90fa70-0c1e-4091-8ba8-501752c927a9n%40googlegroups.com.

[Alexander Nestorov]

Oh! If that is the case then it should be "as simple as" (I hate writing this phrase... every time I write it things get weirdly complex) filter the entire "changed" object when serializing (as you said).

Ping me if you need help with testing the functionality!

Regards

[Roman Bruckner]

It might be even simpler :) We do use only certain properties from the options object. Those defined by `applyOptionsList` and `revertOptionsList`. By default we only need `propertyPath`. So I am thinking of filtering everything else at the time of storing the options (why to keep an ancient cell instance in memory for the whole application life-time). I'll keep you posted.

To view this discussion on the web visit https://groups.google.com/d/msgid/jointjs/fcd33f6c-2879-421b-8da2-027e06202688n%40googlegroups.com.

[Alexander Nestorov]

You mean you'll strip unnecessary stuff from the objects stored in the undo/redoStack? Interesting! This will probably reduce memory leaking (as the CM won't be holding references to unused cells)