Refactor: Change SequenceDB to class based architecture. #6272

saurabhg772244 · 2025-02-12T11:07:13Z

📑 Summary

Refactors the existing sequenceDb implementation from a functional approach to a class-based architecture to improve code organization, maintainability, and extensibility.
Return a new DB object when diagramObject.db is called.
Introduces new SequenceDB class to encapsulate database operations and state
Migrates existing functions to class methods
Maintains existing API interface for backwards compatibility
Following functions still uses common DB:
- apply
- parseMessage
- parseBoxData
- LINETYPE
- PLACEMENT
- setDiagramTitle
- setAccTitle
- setAccDescription
Reason why above functions still uses common DB: Refactor: Change flowDB to class based architecture. #6161 (comment)
Added unique IDs for messages during parsing. Previously, IDs were assigned during rendering, which caused issues for diagrams since message operations relied on IDs.
Updated unit tests to support unique DB instances instead of shared DB.

Resolves #

📏 Design Decisions

The sequenceDb module previously relied on functions and global variables. Consequently, each call to mermaid.parse or mermaid.render resulted in changes to global variables.
This behavior caused issues when rendering multiple diagrams using the same Mermaid API, as the database object (DB) for the diagrams would get mixed up.
To address this, sequenceDb was refactored into a class-based structure. This change ensures that each diagramObject.db call creates a new instance of the database (DB), preventing conflicts between diagrams.

📋 Tasks

Make sure you

📖 have read the contribution guidelines
💻 have added necessary unit/e2e tests.
📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
🦋 If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

…to messages during parsing.

…saurabh/refactor/convert-sequenceDb-to-class

changeset-bot · 2025-02-12T11:07:17Z

🦋 Changeset detected

Latest commit: 451c886

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
mermaid	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

netlify · 2025-02-12T11:07:31Z

✅ Deploy Preview for mermaid-js ready!

Name	Link
🔨 Latest commit	`451c886`
🔍 Latest deploy log	https://app.netlify.com/sites/mermaid-js/deploys/67af496ee39300000844e3ea
😎 Deploy Preview	https://deploy-preview-6272--mermaid-js.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

pkg-pr-new · 2025-02-12T11:08:28Z

Open in Stackblitz

npm i https://pkg.pr.new/mermaid-js/mermaid@6272

npm i https://pkg.pr.new/mermaid-js/mermaid/@mermaid-js/layout-elk@6272

npm i https://pkg.pr.new/mermaid-js/mermaid/@mermaid-js/parser@6272

npm i https://pkg.pr.new/mermaid-js/mermaid/@mermaid-js/mermaid-zenuml@6272

commit: 451c886

packages/mermaid/src/diagrams/sequence/sequenceDb.ts

+    // JSON.parse the text
+    try {
+      let sanitizedText = sanitizeText(text.text, getConfig());
+      sanitizedText = sanitizedText.replace(/&amp;/g, '&');


To fix the problem, we should remove the unnecessary replacements of & and = after the sanitizeText function call. This ensures that the text remains properly sanitized and prevents any double unescaping issues.

packages/mermaid/src/diagrams/sequence/sequenceDb.ts

+      const links: Record<string, string> = {};
+      let sanitizedText = sanitizeText(text.text, getConfig());
+      const sep = sanitizedText.indexOf('@');
+      sanitizedText = sanitizedText.replace(/&amp;/g, '&');


To fix the problem, we need to ensure that unescaping operations are not performed redundantly. The best way to achieve this is to avoid manual unescaping and rely on a well-tested library for sanitization and unescaping. This will help prevent double unescaping and ensure that the data is processed correctly.

We will remove the manual unescaping operations and rely on the sanitizeText function to handle any necessary unescaping. If sanitizeText does not perform unescaping, we should update it to do so.

codecov · 2025-02-12T11:10:05Z

Codecov Report

Attention: Patch coverage is 0% with 581 lines in your changes missing coverage. Please review.

Project coverage is 4.47%. Comparing base (8dd0e7a) to head (451c886).
Report is 11 commits behind head on develop.

Files with missing lines	Patch %	Lines
...ckages/mermaid/src/diagrams/sequence/sequenceDb.ts	0.00%	567 Missing ⚠️
...s/mermaid/src/diagrams/sequence/sequenceDiagram.ts	0.00%	14 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           develop   #6272   +/-   ##
=======================================
  Coverage     4.47%   4.47%           
=======================================
  Files          385     384    -1     
  Lines        54191   54179   -12     
  Branches       598     623   +25     
=======================================
  Hits          2425    2425           
+ Misses       51766   51754   -12

Flag	Coverage Δ
unit	`4.47% <0.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
.../mermaid/src/diagrams/sequence/sequenceRenderer.ts	`0.00% <ø> (ø)`
...s/mermaid/src/diagrams/sequence/sequenceDiagram.ts	`0.00% <0.00%> (ø)`
...ckages/mermaid/src/diagrams/sequence/sequenceDb.ts	`0.00% <0.00%> (ø)`

... and 1 file with indirect coverage changes

aloisklink

The this.setWrap(getConfig().wrap); in the constructor seems to be the only thing I'm a bit nervous about, the rest of the PR seems good to me (as long as the visual regression tests come back green ✔️)

aloisklink · 2025-02-13T15:01:00Z

packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js

+    const diagram1 = await Diagram.fromText(str);
+
+    await diagram1.renderer.draw(str, 'tst', '1.2.3', diagram1); // needs to be rendered for the correct value of visibility auto numbers
+    expect(diagram1.db.showSequenceNumbers()).toBe(true);


suggestion(super non-blocking): I wonder if it's easier to just delete the let diagram at

mermaid/packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js

Lines 94 to 96 in f454865

// const parser = sequence.parser;

let diagram;

Because then the test would be:

const diagram = await Diagram.fromText(str); await diagram.renderer.draw(str, 'tst', '1.2.3', diagram); // needs to be rendered for the correct value of visibility auto numbers expect(diagram.db.showSequenceNumbers()).toBe(true);

So you'll be changing much less lines of code in this PR.

aloisklink · 2025-02-13T15:02:51Z

packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js

@@ -1917,10 +1847,11 @@ Note left of Alice: Bob thinks
 Bob->>Alice: Fine!`;

    await mermaidAPI.parse(str);


todo: There's a couple of mermaidAPI.parse calls you can delete from these unit tests, since they don't do anything.

Suggested change

await mermaidAPI.parse(str);

packages/mermaid/src/diagrams/sequence/sequenceDb.ts

saurabhg772244 · 2025-02-14T12:43:16Z

@aloisklink i think we can refactor it into separate PR

saurabhg772244 added 6 commits January 15, 2025 15:37

convert sequenceDb to class.

5d3d104

Refactored arrow function to regular function and assigned unique ID …

d83b1a5

…to messages during parsing.

Updated unit tests to reflect new DB instance per parse.

f454865

added changeset

ffa7804

Merge branch 'develop' of https://github.com/mermaid-js/mermaid into …

b2dfbb6

…saurabh/refactor/convert-sequenceDb-to-class

Updated unit tests.

6ca8f46

github-actions bot added the Type: Other Not an enhancement or a bug label Feb 12, 2025

github-advanced-security bot found potential problems Feb 12, 2025

View reviewed changes

saurabhg772244 requested review from knsv, sidharthv96 and aloisklink February 12, 2025 12:56

aloisklink reviewed Feb 13, 2025

View reviewed changes

Move setWrap below clear in constructor.

0a1a112

saurabhg772244 requested a review from aloisklink February 14, 2025 12:43

saurabhg772244 marked this pull request as ready for review February 14, 2025 13:09

Code refactor.

451c886

@@ -398,5 +398,3 @@
                 try {
-                  let sanitizedText = sanitizeText(text.text, getConfig());
-                  sanitizedText = sanitizedText.replace(/&amp;/g, '&');
-                  sanitizedText = sanitizedText.replace(/&equals;/g, '=');
+                  const sanitizedText = sanitizeText(text.text, getConfig());
                   const links = JSON.parse(sanitizedText);
@@ -414,6 +412,4 @@
                   const links: Record<string, string> = {};
-                  let sanitizedText = sanitizeText(text.text, getConfig());
+                  const sanitizedText = sanitizeText(text.text, getConfig());
                   const sep = sanitizedText.indexOf('@');
-                  sanitizedText = sanitizedText.replace(/&amp;/g, '&');
-                  sanitizedText = sanitizedText.replace(/&equals;/g, '=');
                   const label = sanitizedText.slice(0, sep - 1).trim();

@@ -398,5 +398,3 @@
                 try {
-                  let sanitizedText = sanitizeText(text.text, getConfig());
-                  sanitizedText = sanitizedText.replace(/&amp;/g, '&');
-                  sanitizedText = sanitizedText.replace(/&equals;/g, '=');
+                  const sanitizedText = sanitizeText(text.text, getConfig());
                   const links = JSON.parse(sanitizedText);
@@ -414,6 +412,4 @@
                   const links: Record<string, string> = {};
-                  let sanitizedText = sanitizeText(text.text, getConfig());
+                  const sanitizedText = sanitizeText(text.text, getConfig());
                   const sep = sanitizedText.indexOf('@');
-                  sanitizedText = sanitizedText.replace(/&amp;/g, '&');
-                  sanitizedText = sanitizedText.replace(/&equals;/g, '=');
                   const label = sanitizedText.slice(0, sep - 1).trim();

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: Change SequenceDB to class based architecture. #6272

Refactor: Change SequenceDB to class based architecture. #6272

saurabhg772244 commented Feb 12, 2025

changeset-bot bot commented Feb 12, 2025 •

edited

Loading

netlify bot commented Feb 12, 2025 •

edited

Loading

pkg-pr-new bot commented Feb 12, 2025 •

edited

Loading

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

codecov bot commented Feb 12, 2025 •

edited

Loading

aloisklink left a comment

aloisklink Feb 13, 2025

aloisklink Feb 13, 2025

saurabhg772244 commented Feb 14, 2025

		@@ -1917,10 +1847,11 @@ Note left of Alice: Bob thinks
		Bob->>Alice: Fine!`;

		await mermaidAPI.parse(str);

Refactor: Change SequenceDB to class based architecture. #6272

Are you sure you want to change the base?

Refactor: Change SequenceDB to class based architecture. #6272

Conversation

saurabhg772244 commented Feb 12, 2025

📑 Summary

📏 Design Decisions

📋 Tasks

changeset-bot bot commented Feb 12, 2025 • edited Loading

🦋 Changeset detected

netlify bot commented Feb 12, 2025 • edited Loading

✅ Deploy Preview for mermaid-js ready!

pkg-pr-new bot commented Feb 12, 2025 • edited Loading

codecov bot commented Feb 12, 2025 • edited Loading

Codecov Report

aloisklink left a comment

Choose a reason for hiding this comment

aloisklink Feb 13, 2025

Choose a reason for hiding this comment

aloisklink Feb 13, 2025

Choose a reason for hiding this comment

saurabhg772244 commented Feb 14, 2025

changeset-bot bot commented Feb 12, 2025 •

edited

Loading

netlify bot commented Feb 12, 2025 •

edited

Loading

pkg-pr-new bot commented Feb 12, 2025 •

edited

Loading

codecov bot commented Feb 12, 2025 •

edited

Loading