Move latestId into editor

[apatton]

Moved node id assignment into editor class to preserve numbering across multiple editors.

[saalihou]

To give more info on the purpose of this change, we have an app where multiple editors can be open at the same time or the user can navigate back and forth between editors in the same session. The static way latestId was implemented before it would always get incremented even when you are in a different editor, which means you could potentially start a new editor and have the first node id be 105, which does not impair the functionality at all but just seemed inconsistent. Hopefully that makes it more clear.

[Ni55aN]

The static way latestId was implemented before it would always get incremented even when you are in a different editor start a new editor and have the first node id be 105

What problems does this lead to?

[saalihou]

We display the id to our users for debugging purposes and this behaviour seems to confuse them as whenever they start working in a new editor they expect the first node to have id 1.

[Ni55aN]

LatestId, etc. should not be placed in the Editor class, since it is not associated with the Editor (it is not the identifier of the editor)

[saalihou]

No it is not the identifier of the editor, but since its purpose is to make sure that no duplicate node id exists within ONE editor, I see it it as the identifier of the latest node in the editor.

[Ni55aN]

There is a flexible solution without needs to modify the core

// isolated-id-plugin.js
export default {
    install(editor) {
        let latestId = 0;

        editor.on('import', ({ nodes }) => {
            latestId = Math.max(...Object.keys(nodes));
        });

        editor.on('nodecreate', node => {
            if (editor.silent) return; // when importing

            node.id = ++latestId;
        });
    }
}

import IsolatedIdPlugin from `isolated-id-plugin`;

editor.use(IsolatedIdPlugin);

This way you can replace id as you need. For example, UUID:

editor.on('nodecreate', node => {
    if (editor.silent) return; // when importing

    node.id = getUUID();
});

[saalihou]

Hi, thank you for the suggestion we ended up implementing the plugin. For future PR's, are there any reasons you don't want the core to be modified ? Is it because there are plugins that rely on the static Node.latestId field ?

[Ni55aN]

@saalihou I prefer to keep the core as simple as possible. Therefore, if most developers dont require such a modification, it should be implemented as a plugin.