Skip to content
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

Serializable Diagram State #362

Open
loreanvictor opened this issue Jun 24, 2024 · 0 comments
Open

Serializable Diagram State #362

loreanvictor opened this issue Jun 24, 2024 · 0 comments

Comments

@loreanvictor
Copy link
Contributor

loreanvictor commented Jun 24, 2024

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);
  }
}
  • This solution effectively results in some pseudo-implementation of OOP in a customised manner. Tracing deeper inheritance chains would be difficult and complex, but simpler chains would be easily resolved.
  • As it currently stands, most element and relationship classes do not add any specific functionality except for their custom component, which is already registered with a similar mechanism and not via class inheritance. This change might reduce the size of the code base drastically.
  • Since Apollon code base is in TypeScript, this solution would also require equivalent types and interfaces for various element and relationship types (where currently we have classes).
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

No branches or pull requests

1 participant