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.

Sign up for GitHub

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

Jump to bottom

[Architecture/fix]: Better documentation & error handling of issues causing inconsistent rendering #6230

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

[Architecture/fix]: Better documentation & error handling of issues causing inconsistent rendering #6230

wants to merge 8 commits into from

Conversation

NicolasNewman
Copy link
Collaborator

📑 Summary

Handles issues that caused architecture diagrams to have non-deterministic rendering. The underlying cause was due to 1 of 2 issues:

  1. Poorly documented limitation with the underlying rendering engine (Architecture diagram - different layout rendering each time #6166)
  2. Missing constraints for some cases of XY edges (non-deterministic architecture-beta rendering #6024)

Resolves #6024, #6166

📏 Design Decisions

Missing constraints (6024)

  • getRelativeConstraints() has been updated to guarantee it handles all directions for XY edges.
  • A defect was found in shiftPositionByArchitectureDirectionPair() where the Y shift for XY edges was returning the incorrect sign. Since getRelativeConstraints() wasn't properly handling all XY edges in the first place, the issue never manifested itself sooner.

Poorly documented limitations (6166)

An underlying limitation with the rendering engine is XY edges cannot extend more then 1 unit in the X/Y direction.

  • A new section has been added to the documentation explaining this, along with common ways to fix it.
  • An error is explicitly thrown if a jump that is > 1 is found.

📋 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:.

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jan 27, 2025

🦋 Changeset detected

Latest commit: 9531cd3

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 Netlify
Copy link

netlify bot commented Jan 27, 2025 •
edited
Loading

✅ Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 9531cd3
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/6797ee23b5a75300084c8865
😎 Deploy Preview https://deploy-preview-6230--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@NicolasNewman NicolasNewman linked an issue Jan 27, 2025 that may be closed by this pull request
Architecture diagram - different layout rendering each time #6166
Open
@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Jan 27, 2025

Open in Stackblitz

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

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

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

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

commit: 9531cd3

@codecov Codecov
Copy link

codecov bot commented Jan 27, 2025 •
edited
Loading

Codecov Report

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

Project coverage is 4.46%. Comparing base (e2de552) to head (9531cd3).

Files with missing lines Patch % Lines
.../src/diagrams/architecture/architectureRenderer.ts 0.00% 46 Missing ⚠️
...ermaid/src/diagrams/architecture/architectureDb.ts 0.00% 42 Missing ⚠️
...aid/src/diagrams/architecture/architectureTypes.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #6230      +/-   ##
==========================================
- Coverage     4.47%   4.46%   -0.01%     
==========================================
  Files          384     384              
  Lines        54179   54261      +82     
  Branches       623     623              
==========================================
  Hits          2425    2425              
- Misses       51754   51836      +82
Flag Coverage Δ
unit 4.46% <0.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
...aid/src/diagrams/architecture/architectureTypes.ts 0.00% <0.00%> (ø)
...ermaid/src/diagrams/architecture/architectureDb.ts 0.00% <0.00%> (ø)
.../src/diagrams/architecture/architectureRenderer.ts 0.00% <0.00%> (ø)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sidharthv96 sidharthv96 Awaiting requested review from sidharthv96

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Architecture diagram - different layout rendering each time non-deterministic architecture-beta rendering
1 participant
@NicolasNewman