projectstorm / react-diagrams

a super simple, no-nonsense diagramming library written in react that just works
https://projectstorm.cloud/react-diagrams
MIT License
8.71k stars 1.18k forks source link

Deserialization isn't working properly #997

Open Nicocchi opened 1 year ago

Nicocchi commented 1 year ago

I've followed the example serialization code, and serialization works, but when I try to deserialize, if more than 2 ports are added, it just returns: Uncaught TypeError: Cannot read porperties of null (reading 'getID') at DefaultNodeModel.ts:114:17 I'm not using any custom nodes or links, etc just DefaultNodeModel.

Stack: React 18.2 + Vite 4.1 + Electron 23.2

Serialization

const data = currentModel.serialize();
// Send data to ipcRender to save to file
try {
    const res = await window.ipcRenderer.sendSync("save-project", {data });
} catch (error) {
    console.error(error);
}

Deserializaton

var model2 = new DiagramModel();
// I tried this since I saw a comment with looking at the programmatic demo
let obj: ReturnType<DiagramModel['serialize']> = JSON.parse(res);
model2.deserializeModel(obj, engine);
engine.setModel(model2);

// just sets my state's current model
setCurrentModel(model2);

Adding ports

 _.forEach(
    currentModel.getSelectedEntities(),

    (item: BaseModel<any>) => {
        item.addInPort(`${fname}`, true);
        item.addOutPort(`out-${item.getOutPorts().length + 1}`, true);
    }
);

engine.repaintCanvas();

I am also saving and opening the data to a JSON file

Saving

const data = JSON.stringify(arg.data);
fs.writeFile(`${result.filePath}.json`, data, (err) => {
    if (err) {
        console.log(err);
        return (_.returnValue = false);
    } else {
        console.log("File written successfully\n");
        console.log("The written has the following contents:");
        console.log(fs.readFileSync(`${result.filePath}.json`, "utf-8"));
    }
})

Opening

fs.readFile(res.filePaths[0], "utf-8", (err, data) => {
    if (err) {
        console.log(err);
        return (_.returnValue = null);
    } else {
        console.log("Contents of the readFile are:\n");
        console.log(data);
        return (_.returnValue = data);
    }
})
Alletkla commented 1 year ago

Hey you just swapped name and id in the constructor of your Port.

Since the name of the port is used as an index in NodeModel (which DefaultNodeModel relies on, when u watch in the implementation):

addPort(port) {
        port.setParent(this);
        this.ports[port.getName()] = port;
        return port;
    }

So essentially only add 1, all further are overwritten, since you use "true" as name.

And now the big BUT: DefaultNodeModel also saves its in and outPorts BUT: in lists Thats why all is displayed properly (since the DefaultNodeWidget relies on the lists, but serializing and deserializing breaks the code since:

deserialize(event) {
        super.deserialize(event);
        this.options.name = event.data.name;
        this.options.color = event.data.color;
        this.portsIn = _.map(event.data.portsInOrder, (id) => {
            return this.getPortFromID(id);
        });
        this.portsOut = _.map(event.data.portsOutOrder, (id) => {
            return this.getPortFromID(id);
        });
    }

relies on this.getPortFromID() from NodeModel and this searches its own port object representation for the id, where only the last added port is present. resulting in an null return and writing null in the serialized object.


Bottom line:

Now you know WHY it fails a fix four your issue will be to just swap the parameters in the constructor of your ports.

But: I think my PR (which wasn't meant to be for this) could result in an fix for this. If this would result in an major change of how ports are saved in NodeModel.

1013

Because even if your issue can easily be fixed it shows a major weakness (at least in my eyes) of how ports are handled in NodeModel