feat(data-modeling): export diagram to png COMPASS-9449 #7055

mabaasit · 2025-06-26T08:14:40Z

Description

Checklist

New tests and/or benchmarks are included
Documentation is changed or added
If this change updates the UI, screenshots/videos are added and a design review is requested
I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

Bugfix
New feature
Dependency update
Misc

Open Questions

Dependents

Types of changes

Backport Needed
Patch (non-breaking change which fixes an issue)
Minor (non-breaking change which adds functionality)
Major (fix or feature that would cause existing functionality to change)

…PASS-9449-export-to-png

gribnoysup · 2025-06-26T08:28:39Z

packages/compass-data-modeling/src/services/export-diagram.tsx

+          // So, when exporting, we need to include those defs as well so that
+          // edge markers are exported correctly.


Wouldn't it be easier to hide what we don't need instead of doing this?

Can we make this configurable in the diagramming package?

Wouldn't it be easier to hide what we don't need instead of doing this?

I'll evaluate this, last time (in poc) i checked this, it did not give me good results.

Can we make this configurable in the diagramming package?

These custom marker are rendered here in the package. I am not sure if i follow what you mean by making it configurable?

Sorry, comment somehow attached to the wrong place, should've been this part:

It excludes diagram title, minmap, and other UI elements

We're doing all this to avoid including controls in the diagram screenshot. Can we change diagramming package to allow us to render it with hidden controls? Then you don't need to do anything to target an element that doesn't include everything that you need

ah, yeah that should be possible with the package. but again it depends on if we are able to export the root container (.react-flow) and if not, then maybe we don't need it.

I tried to do this and was not able to get it to export correctly using this approach.

packages/compass-data-modeling/src/services/export-diagram.tsx

mabaasit · 2025-06-26T13:23:07Z

packages/compass-e2e-tests/tests/data-modeling-tab.test.ts

  waitForFileDownload,
 } from '../helpers/downloads';
 import { readFileSync } from 'fs';
+import { recognize } from 'tesseract.js';


For current changes in this file, I ended up doing png export e2e tests in this PR. While exploring OCR testing, wdio relies on tesseract.js. So i added it as a dep and as we are downloading an image, it makes sense to do it this way (as compared to using wdio api to read contents from an image on webpage).

I ran this test many time locally to ensure it does not fail randomly. Collection name changes in this file are mainly because it struggles to read testCollection1 (reads it as testCollectionl)

Also, when @paula-stacho's positioning PR lands, it will ensure that nodes don't overlap and we will not have any unexpected failing assertions.

…PASS-9449-export-to-png

packages/compass-data-modeling/src/components/export-diagram-modal.tsx

Anemy · 2025-07-02T17:25:22Z

packages/compass-data-modeling/src/services/export-diagram.tsx

-): Promise<string> {
-  return new Promise<string>((resolve, reject) => {
+export function getExportPngDataUri(diagram: DiagramInstance): Promise<string> {
+  return new Promise<string>((resolve, _reject) => {


Is there a reason why we're going to the Promise resolve/reject here? I'm thinking it could lead to an uncaught exception. Could we instead have the cleanup in a finally or catch block?

We could do await something like the rafraf as well and await the toPng.

What would you recommend in this case? I only want to resolve once dom has been converted to a data uri (and as such show loading state to the user). For clean up, its already in place where we export.

I would recommend async/await here with try/catch. I find it makes the code easier to read through and the error handling less prone to be uncaught.

packages/compass-data-modeling/package.json

gribnoysup · 2025-07-03T09:01:58Z

packages/compass-data-modeling/src/components/export-diagram-modal.tsx

-  }, [exportFormat, onCloseClick, model, diagramLabel]);
+  }, [onCloseClick]);
+
+  const onExport = useCallback(async () => {


There is way too much logic in the UI here (like if you see that you're keeping abort controllers in render, you probably should start thinking about that), this should really be an action in the store instead

Yeah, i'll add another slice for export. Or, I can do it as a follow up if that sounds okay to you.

Follow-up sounds good, let's not hold this code in the branch for too long 👍

gribnoysup · 2025-07-03T09:03:45Z

packages/compass-data-modeling/src/components/export-diagram-modal.tsx

+  const diagram = useDiagram();
+  const [isExporting, setIsExporting] = useState(false);
+  const abortControllerRef = useRef<AbortController | null>(null);
+  const toast = useToast();


Use our openToast instead, not sure how are we even exporting this one, we really shouldn't

gribnoysup · 2025-07-03T09:50:05Z

packages/compass-data-modeling/src/services/export-diagram.tsx

+    }));
+
+    const reject = (error: Error) => {
+      document.body.removeChild(container);


Seems like we're missing clean-up on resolve, this should probably be in a finally block

ah, @Anemy also mentioned same and that time i completely misunderstood this. i added this in 6136ff4 along with other fixes. Regarding useToast hook, i'll do a follow up to remove it from compass-components as its not being used elsewhere.

Sounds good!

i'll do a follow up to remove it from compass-components

Just FYI, I already did in eslint update PR, so no need to worry about this (was in the way of some new issues that update started to show there)

paula-stacho · 2025-07-03T10:27:02Z

packages/compass-e2e-tests/tests/data-modeling-tab.test.ts

+    expect(text).to.include('jint');
+    // it does not correctly recognize `iString` and only returns `String`.
+    // its already good enough to verify this for now and if it flakes
+    // more, we may need to revisit this test.


that was unlucky naming on our part 😅 it'd be easier to OCR full words rather than lone letters, especially tricky ones like 'i'. now that it's used everywhere, I assume it'd be too much hassle to rename

…PASS-9449-export-to-png

gribnoysup

Looks good taking into account some follow-ups we're planning to do

gribnoysup · 2025-07-03T12:45:20Z

packages/compass-data-modeling/src/services/export-diagram.tsx

+          const viewportElement = container.querySelector(
+            '.react-flow__viewport'
+          );


Nit: instead of relying on arbitrary frames you can probably wait for this element to be in the view (similar to what we do in tests sometimes)

gribnoysup · 2025-07-03T12:45:41Z

packages/compass-data-modeling/src/services/export-diagram.tsx

+    }));
+
+    const reject = (error: Error) => {
+      document.body.removeChild(container);


Nit: you don't need it here now that you have it in finally

mabaasit and others added 20 commits June 17, 2025 13:34

extract diagram editor toolbar:

cce62ac

add export modal

a43ed7e

json export

39793bd

tests

e47687e

close modal

dbc1ca0

Merge branch 'main' into COMPASS-9448-diagram-json-export

6d8de76

tests

087936c

ensure test run

6c21b63

fix toast

b7d8dcf

fix electron test

7c4c14e

fix link

f9bcc51

ensure its thrown

b65f4c8

asset number of selected collections

adc19a1

fix modal styles

6c41226

Merge branch 'main' into COMPASS-9448-diagram-json-export

86e8a92

return null for tests

7fa0480

Merge branch 'main' into COMPASS-9448-diagram-json-export

8c5c14d

remove comment

ffef0b7

export image to png

88a227b

Merge branch 'main' of https://github.com/mongodb-js/compass into COM…

487a2b9

…PASS-9449-export-to-png

github-actions bot added the feat label Jun 26, 2025

gribnoysup reviewed Jun 26, 2025

View reviewed changes

packages/compass-data-modeling/src/services/export-diagram.tsx Outdated Show resolved Hide resolved

move container to the export png

56aadf0

addaleax reviewed Jun 26, 2025

View reviewed changes

packages/compass-data-modeling/src/services/export-diagram.tsx Outdated Show resolved Hide resolved

add ocr e2e test

42b9c8e

mabaasit commented Jun 26, 2025

View reviewed changes

mabaasit added the no release notes Fix or feature not for release notes label Jun 26, 2025

mabaasit added 2 commits June 27, 2025 10:33

abortable export

68d00cf

Merge branch 'main' of https://github.com/mongodb-js/compass into COM…

7b220ca

…PASS-9449-export-to-png

mabaasit and others added 10 commits June 30, 2025 09:52

use package methods

8a08691

clean up

3b16d26

Merge branch 'main' into COMPASS-9449-export-to-png

c035d7a

update diagramming

cda452d

Merge remote-tracking branch 'origin' into COMPASS-9449-export-to-png

a47faf8

npm install

15bf100

npm check

ae8e3c6

fix test

840c078

Merge branch 'main' of https://github.com/mongodb-js/compass into COM…

4bca5a2

…PASS-9449-export-to-png

update npm

0a9a6e4

mabaasit marked this pull request as ready for review July 2, 2025 10:12

mabaasit requested a review from a team as a code owner July 2, 2025 10:12

mabaasit added 3 commits July 2, 2025 17:03

Merge branch 'main' of https://github.com/mongodb-js/compass into COM…

aab9aca

…PASS-9449-export-to-png

fix spaces issue

c6bc709

fix popup for firefox

010b0c6

Anemy reviewed Jul 2, 2025

View reviewed changes

gribnoysup reviewed Jul 3, 2025

View reviewed changes

paula-stacho reviewed Jul 3, 2025

View reviewed changes

mabaasit added 3 commits July 3, 2025 14:03

skip on win and lowercase test assertions

b34c1f5

cr comments

6136ff4

Merge branch 'main' of https://github.com/mongodb-js/compass into COM…

fd4ee26

…PASS-9449-export-to-png

gribnoysup approved these changes Jul 3, 2025

View reviewed changes

npm install

065851a

mabaasit merged commit 0822820 into main Jul 3, 2025
55 of 58 checks passed

mabaasit deleted the COMPASS-9449-export-to-png branch July 3, 2025 14:10

		// So, when exporting, we need to include those defs as well so that
		// edge markers are exported correctly.

feat(data-modeling): export diagram to png COMPASS-9449 #7055

feat(data-modeling): export diagram to png COMPASS-9449 #7055

Uh oh!

Conversation

mabaasit commented Jun 26, 2025

Description

Checklist

Motivation and Context

Open Questions

Dependents

Types of changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gribnoysup left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants