Modularise mermaid to have diagram specific code not spread across the mermaid code base

[financelurker]

Is your feature request related to a problem? Please describe.

When developing a new diagram type for mermaid it is a huge change to the complete mermaid code. Currently there is the diagram code in the diagrams folder itself, and other necessary changes: defaultConfig.js, Diagram.js, mermaidAPI.js, styles.js, utils.js and several locations in the files contained in the themes folder (did I miss something?).

Describe the solution you'd like
The ideal solution - imho - would be that the mermaid project is split up into:

• mermaid API package
• diagram packages (one dedicated package for each mermaid diagram type - could also be bundling more diagram types)
  • which use the mermaid API package to register their styles, theme-extensions, default-configs, logic-functions, ...
• mermaid logic package
  • which also uses the mermaid API package to utilise the previously registered diagrams

In the end the base code of mermaid should be free of any diagram information, can focus on orchestration and must only consume the mermaid API - no hard-coded imports of diagrams and neither should the diagram packages import anything from the mermaid logic package.

Then an using app could "simply" import the mermaid logic package and the supported diagrams (or a diagram-bundle-package).

Describe alternatives you've considered

The currently available alternative is to add new diagram types and have a huge junk of the mermaid code base also changed.

Additional context

• It would also be nice to have the monaco-mermaid declarations integrated into a diagram package, so the diagram package "out-of-the-box" provides auto-completion for apps using the monaco-editor (like the mermaid-live-editor). Tho, that can be a separate task and should be proposed to the monaco-mermaid maintainers/contributors.
• Such a mechanism can be implemented along side the current diagrams (to not break existing apps which already have mermaid integrated). At the latest when the project is split into multiple components the project probably should be renamed to mermaid2 (or have a major version change) to indicate incompatibility to the previous integration.
• Implementing this strategy would also enable/resolve Make mermaid smaller using code splitting #2459 in the future.

Implementation proposal

1. try to get diagram specifics within the diagrams folder without splitting the project (the backward compatible change for currently integrating apps)
   1. identify all necessary information to put into an appropriate "DiagramAPI" object
      1. extended Theme attributes
      2. styles and diagram-scss/css
      3. default-config
      4. diagram type/name
      5. diagram parser
      6. diagram renderer
      7. ...
   2. extend the current logic to use that DiagramAPI object if no default diagram was identified (such in utils.js:detectType, the switch-case in mermaidAPI.js:render and Diagram.js:Diagram.constructor, ...)
   3. try to implement the sequence-diagram with that approach (make it a sequenceDiagram-v2 or so) as a PoC
   4. ... (did I forget something?)
   5. migrate existing diagram types to that approach and consequently cleanup the orchestrating code
2. split the project

---

@financelurker thats a good idea. I think that we are reaching a point where the base package of mermaid needs to be split up. If you don't care about gantt charts why include and load them? I have been thinking about a lazy loading. This could be another way or even better a part of that setup.

There are some refactoring activities that need to happen first though. The code that initially handled two diagrams is not as clean when handling 10+ diagrams after"organic growth". We need to define what a diagram is so that mermaid can treat them all diagrams in the same way with a basic interface. This is high up on my prioritisation list and diagram organisation is a part of that.

Originally posted by @knsv in #2456 (comment)

[financelurker]

In that matter, having a look across the theme artefacts I stumbled over the following thing:

• For sequence diagrams the variable sequenceNumberColor is defined in theme-*.js files and also in the */index.scss files (with the same values)... Is that a wanted redundancy?

And, how could a diagram provide it's own scss at run-time and not at compile-time? (is there a solution for that?)

[financelurker]

One other finding is:
There are two unit tests in mermaidAPI.spec.js checking that the "global config" isn't changed somewhere within the runtime. Why is that and what is the implication of runtime-changes in the config object? In a local test-implementation I have disabled these tests, since diagrams need to provide their own config to the defaultConfig object (or do they? this can probably be refactored as well within the configAPI) and thereby will alter/amend that object...

On second thought there should be no need for a diagram to extend the "defaultConfig" object... If the parser/db/renderer of a diagram needs to access it's config it should be able to import it directly. Or am I missing something?

[knsv]

@financelurker No there are artifacts from organic growth in the code. It is high time for some refactoring. I am looking forward to start with that. The first step will be to create the abstract diagram which opens things up. I think we need to code cleaning to happen as well. 😊 We have some commented out code thats should be removed, etc.

The sass stuff should also be removed as it is not used anymore (#3093 3093).

Regarding config it has to do with the directives where you can change some of the configuration options from the diagram code. You don't want the directives changes to the configuration to pollute the global configuration. It is surprisingly complicated.

[knsv]

Good points regarding with gathering everything you need to be a diagram into each diagram.

[financelurker]

@knsv Ad "example implementation": Is there a compatibility reason to not use ES6 features? Because I'd like to see JavaScript classes that provide a kind of standardised interface for "diagrams", so it's easier for developers to know what a custom diagram has to provide to the orchestrating logic.

Ad "configuration": Okay, so there's three layers of configuration: the "mermaid default config", the "diagram default config" and on top of that the configuration provided via directives? I think this complexity should be hidden within the configAPI somehow - which could be also orchestrated within the mermaidAPI, since it's providing the parseDirective/handleDirective functions which the diagram parser are reusing.
Is utils.assignWithDepth probably the function, which merges the default config and the diagram config to a new config-object? (looks to me like that, because it gets invoked by configAPI.getConfig and the comment of utils.assignWithDepth tells me that this function copies the diagram config to the top-level, which provides the functionality that a diagram config can override a default config - is this a correct interpretation of how this works?)

[knsv]

Yes, the extension of the default config is used in order to have a whitelist of configuration options so what the directives can not add anything else. This caused security issues.

Using es6 for the standardised diagram is my though as well. The interface should have a way of registering valid configuration options.
Also agree with @Yash-Singh1 in that we should have a monorepo for the diagrams. Apart from yarn workspaces we also have pnpm workspaces as an option.

[financelurker]

So, I have some assumptions about the current config-code, which tackles these security issues:

• the defaultConfig.secure attribute declares which object attributes of the default config cannot be overridden
  • thoughts/questions: does that target only config values provided by directives? Or is it allowed for a diagram to provide a secure attribute and therefore override the defaultConfig.secure values? Because I haven't seen any protection of the secure attribute within the utils.assignWithDepth function. That would be problematic if a custom diagram provides this attribute and unlocks the change of previously unchangeable attributes. If there's no need for a diagram to override existing secure attributes the utils.assignWithDepth could be extended to only add additional values to that array and prevent the removal of existing values...
• the configAPI.sanitize function checks and prevents executable code which is provided in the config (to prevent malicious diagram config provided by the diagram author)

Are there other aspects of security for now, which I haven't found by now?

Which makes me wonder, in a scenario where diagrams can be loaded as dedicated npm packages into the using-app-runtime: What strategies would mermaid need to implement, to protect a consuming app if they load "unknown" or "unverified" diagram-packages, which injects malicious code? That would be a new attack vector for mermaid users...
Or would it then be declared as "it's the responsibility of the app-developer which diagrams he's bundling within his web-app"?

---

Ad "monorepo": This would fit the development of default diagrams for mermaid, I guess... Custom diagrams, which companies will develop for themselves or their customers will stay in their code repositories and will then be bundled in their apps. (or other wild thought: they'll have dedicated own GitHub-/*-repositories until they get incorporated into mermaid)
But, is a monorepo that suitable? Wouldn't it be "nicer" for each diagram type to have it's own version tree/issue tracking/etc. or do you think that this would be too much organisational effort?

[knsv]

I think defaultConfig.secure should only be allowed to be added to by a custom diagram ... if even that. I think we could aim for this not being allowed until convinced it is required.

The general security question is important. In the end we cant get away from that the final responsibility will land on the "integrator" that integrated mermaid in an app or on a site. All that said, we should have solid foundation for the security, can we help in that effort we should though.

I think a monorepo is that way to go for the diagrams that mermaid develops as it, in my experience, greatly reduces the administration and development speed.

Another aspect that very important that need to be considered is backwards compatibility. It is unfortunate if changes in the API renders lots of custom diagrams defunct. Before external v1 on the API, open for external use we should be confident that API is solid.

[weedySeaDragon]

Just a suggestion: This thread is an excellent candidate to move to 'Discussions' since it cuts across so many things.
Would also be helpful to make some project milestones out of some of these as they become clear.

Speaking as someone coming in to the project and trying to figure out what is being worked on, etc. :-)

I also recognize that it causes a disruption to do so (people involved in this thread are already used to watching and participating in it; PRs/commits can easily point here). Maybe just a single mention of it in a Discussion to point to this thread would be worthwhile.