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

Move ELK to standalone package #5049

Merged
merged 13 commits into from
Nov 24, 2023
Merged

Move ELK to standalone package #5049

merged 13 commits into from
Nov 24, 2023

Conversation

sidharthv96
Copy link
Member

@sidharthv96 sidharthv96 commented Nov 20, 2023

📑 Summary

Splits Flowchart-ELK into a separate package, to make licensing simpler.

Resolves #5043

This allows sites to fallback to dagre layout for flowchart-elk if the elk package is not installed, instead of completely breaking rendering.
We need to pick either #5050 or #5051 for the approach to override.

📏 Design Decisions

Moved files to a new package, removed elkjs dependency from the mermaid core.
Edited tsconfig to allow import of files from mermaid, to avoid duplicating code.

📋 Tasks

Make sure you

Copy link

netlify bot commented Nov 20, 2023

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 5eb1160
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/656035dc6dbf5700081baad4
😎 Deploy Preview https://deploy-preview-5049--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.

Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Merging #5049 (bcdd1c5) into next (b48136d) will decrease coverage by 2.21%.
Report is 577 commits behind head on next.
The diff coverage is 40.50%.

❗ Current head bcdd1c5 differs from pull request most recent head 5eb1160. Consider uploading reports for the commit 5eb1160 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #5049      +/-   ##
==========================================
- Coverage   46.57%   44.37%   -2.21%     
==========================================
  Files          55       25      -30     
  Lines        6819     5217    -1602     
  Branches       36       25      -11     
==========================================
- Hits         3176     2315     -861     
+ Misses       3642     2901     -741     
  Partials        1        1              
Flag Coverage Δ
unit 44.37% <40.50%> (-2.21%) ⬇️

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

Files Coverage Δ
packages/mermaid/src/config.ts 52.22% <100.00%> (-8.10%) ⬇️
packages/mermaid/src/defaultConfig.ts 45.05% <100.00%> (+0.81%) ⬆️
packages/mermaid/src/diagram-api/detectType.ts 67.07% <0.00%> (+0.80%) ⬆️
packages/mermaid/src/diagrams/gantt/ganttDb.js 76.67% <60.00%> (+0.07%) ⬆️
packages/mermaid/src/diagrams/common/commonDb.ts 87.50% <87.50%> (ø)
packages/mermaid/src/diagram-api/diagramAPI.ts 58.82% <43.47%> (+3.12%) ⬆️
packages/mermaid/src/diagrams/common/common.ts 52.59% <19.78%> (-11.56%) ⬇️

... and 38 files with indirect coverage changes

Copy link
Member

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this approach!

The only thing I'd like to see if some E2E tests to make sure that flowchart-elk still renders correctly on the base Mermaid and when import flowchartELK from './mermaid-flowchart-elk.esm.mjs'; is run

package.json Show resolved Hide resolved
packages/mermaid-flowchart-elk/package.json Outdated Show resolved Hide resolved
packages/mermaid-flowchart-elk/package.json Outdated Show resolved Hide resolved
* next: (39 commits)
  reset the testTimeout to 5 seconds and change it directly in the test
  update testTimeout from 5 seconds to 10 seconds
  Update all patch dependencies
  fix broken link
  add latest blog post
  Update all minor dependencies
  fix linting
  fix: set proper shiki theme for light and dark modes
  fix: change shiki theme to github-light
  add latest blog post
  GitGraph: made reroute fn more readable
  GitGraph: simplified branch check in arrow rerouting fn
  GitGraph: added commit IDs to e2e test to remove false positives
  GitGraph: Moved branch curve check to within reroute check fn
  GitGraph: corrected minor typo in comment.
  GitGraph: added 2x e2e tests for branches not used immediately
  GitGraph: added branch checking to rerouting
  GitGraph: Added e2e tests for deferred branch use.
  GitGraph: e2e tests, added commit IDs to test graphs
  GitGraph: fixed an e2e branch for vertical branch
  ...
@sidharthv96 sidharthv96 merged commit 6d49cd6 into next Nov 24, 2023
22 of 23 checks passed
@sidharthv96 sidharthv96 deleted the sidv/splitELK branch November 24, 2023 05:35
@sidharthv96 sidharthv96 linked an issue Nov 24, 2023 that may be closed by this pull request
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

Successfully merging this pull request may close these issues.

Move ELK to external package
2 participants