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

Release/10.0.0 #4108

Merged
merged 41 commits into from
Feb 20, 2023
Merged

Release/10.0.0 #4108

merged 41 commits into from
Feb 20, 2023

Conversation

sidharthv96
Copy link
Member

@sidharthv96 sidharthv96 commented Feb 19, 2023

📑 Summary

PSA: Don't get overwhelmed by the files changed.

Most are test changes, as almost all E2E tests and many unit tests had to be modified because the render type changed.
Then there are lots of small import changes.

Only 3 files have significant changes.

  • packages/mermaid/src/Diagram.ts
  • packages/mermaid/src/mermaid.ts
  • packages/mermaid/src/mermaidAPI.ts

Breaking changes planned in v10

Fixes #3590
Fixes #3577
Fixes #3394
Fixes #3406
Fixes #3055


Optional - Could be pushed to v11 as it's going to be a massive change including diagram rendering code.

📋 Tasks

Make sure you

  • 📖 have read the contribution guidelines
  • 💻 have added unit/e2e tests (if appropriate)
  • 📓 have added documentation (if appropriate)
  • 🔖 targeted master branch

renovate bot and others added 22 commits February 9, 2023 05:54
chore(deps): update dependency vite to v4
* master: (24 commits)
  docs: fix links
  Skip precommit hooks on CI
  Fix release-publish
  Fix timeline and mindmap
  Updating integration instructions for timeline and mindmap
  Remove node heap
  Revert "chore: Set node heap size"
  Revert "Remove text hint"
  Split cytoscape
  Linear build
  Remove text hint
  Fix elk import
  Dynamic elk import
  Remove heap option
  elk web-worker
  Test publish docs
  chore: Add file extension for dynamic import
  chore: Defer elk loading
  Update vitepress
  Fix links to integrations.md
  ...
chore(deps): update dependency cypress to v12
Manually tree shaking import statement of package.json
* develop:
  Add highlight tag info in contributing.md
  chore(deps): update dependency cypress to v12
  docs: fix links
  Fix types
  chore(deps): update dependency vite to v4
* release/9.4.2:
  RC version
  Revert #4034
  Revert #4034
  fix: Vite, D3, Vitest Types
fix(api): tree shaking package.json import
* release/9.4.2:
  RC version
  Revert #4034
  Revert #4034
  fix: Vite, D3, Vitest Types
  fix(api): tree shaking package.json import
Both render and parse are async now.
Return type of render contains svg and bindFunctions.
Parse will not throw error if parseOptions.silent is passed.
@sidharthv96
Copy link
Member Author

Sneakpeek esbuild

❯ time nr esbuild
nr esbuild  3.75s user 0.54s system 317% cpu 1.351 total

# Current
❯ time nr -w build
nr -w build  31.55s user 3.37s system 166% cpu 21.035 total

@sidharthv96 sidharthv96 linked an issue Feb 19, 2023 that may be closed by this pull request
@sidharthv96 sidharthv96 linked an issue Feb 20, 2023 that may be closed by this pull request
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 probably won't have time for a while to do a full review, but as a temporary review, all the breaking changes look good to me! 🚀

However, I think we need a bit more documentation in V10-BreakingChanges.md

Edit: One potential issue with the migration to ESM is that some projects that use Mermaid are still not on ESM (e.g. docusaurus). This means that we might need to backport security fixes if people are still using v9.x and can't upgrade to v10.


## Async

`parse`, `render` are now async.
Copy link
Member

Choose a reason for hiding this comment

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

It might be worth having some examples here, since this is probably one of the most important changes!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I'll add documentation soon (with examples and live editor commits for migration)
This file is currently used to keep note of points to elaborate before release.

packages/mermaid/src/mermaid.ts Outdated Show resolved Hide resolved
@@ -1,5 +1,58 @@
# A collection of updates that change the behaviour
# A collection of updates that change the behavior

Copy link
Member

Choose a reason for hiding this comment

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

We need to document that Mermaid is now ESM only.

Ideally we should show some example usage too.

Co-authored-by: Alois Klink <alois@aloisklink.com>
/**
* @deprecated This is an internal function and should not be used. Will be removed in v10.
*/
export interface RenderResult {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move this declaration to top of file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

try {
// diag = new Diagram(text);
diag = getDiagramFromText(text);
if ('then' in diag) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that the if('then' in diag) { ... on line 476 on current file is no longer present. Not needed any longer?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, that was used to check whether it's a promise or not. Now, it'll always be a promise.
Nice catch!

Copy link
Contributor

@pbrolin47 pbrolin47 left a comment

Choose a reason for hiding this comment

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

A few comments on packages/mermaid/src/Diagram.ts,
packages/mermaid/src/mermaid.ts,
packages/mermaid/src/mermaidAPI.ts

Co-authored-by: Per Brolin <per@mermaidchart.com>
@pbrolin47 pbrolin47 merged commit fb8572f into master Feb 20, 2023
@pbrolin47 pbrolin47 deleted the release/10.0.0 branch February 20, 2023 12:28
@sidharthv96 sidharthv96 restored the release/10.0.0 branch February 20, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment