ls1intum / Apollon

UML Modeling Editor written in React
https://apollon-library.readthedocs.io
MIT License
61 stars 21 forks source link

Serializable Diagram State #362

Open loreanvictor opened 3 weeks ago

loreanvictor commented 3 weeks ago

Is your feature request related to a problem?

We currently store non-serializable class instances inside the Redux store (e.g. here and here). That is because we encode specific behaviour of each element type / relationship type of each diagram type using a dedicated class (like this) and have generally organised such functionality over a hierarchy-tree encoded with inheritance.

❌❌ WE SHOULD NOT BE DOING THIS. Redux explicitly lists this requirement as an essential one:

Do Not Put Non-Serializable Values in State or Actions.

Avoid putting non-serializable values such as Promises, Symbols, Maps/Sets, functions, or class instances into the Redux store state or dispatched actions.

As explained in this answer, unfortunately this rule also means use of inheritance, as currently implemented in Apollon, is also not possible while following Redux's essential guidelines.

Current implementation, in some situations, specifically leads to redundant re-rendering, as the whole instance is replaced with a new one and redux is unable to trace down the changes. It is also a potential source for other issues (such as debug tools not working properly) and footguns (methods that mutate objects having apparently no effect).

Describe the solution you'd like

We should keep plain objects in Redux.

This can be achieved by moving functionality of current class methods to some standalone helper functions, taking the instance as their first argument, and producing the necessary result. For each type-agnostic functionality (i.e. method of a parent class), we can have helper functions routing based on element (or relationship) type to the type-specific implementation, or fallback to a more generic implementation working for a wider range of element (or relationship) types.

For example, the custom render() method of BPMNSwimlane can be a function registered for this particular element type, and another helper function will try to retrieve the type-specific render function for each element type, falling back to a more generic one (or in this case, simply noop since render() doesn't do anything except when it is overriden).

export function renderBPMNSwimlane(element: BPMNSwimlane) {
  // ...
}
export renderFunctions = {
  // ...
  [BPMNElementType.BPMNSwimlane]: renderBPMNSwimlane,
  // ...
};

export function render(element: UMLElement) {
  const candidateRender = renderFunctions[element.type];
  if (candidate) {
    return candidateRender(element);
  } else {
    return genericRender(element);
  }
}