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

Add directive support to all diagrams by preprocessing #4759

Merged
merged 10 commits into from
Sep 7, 2023

Conversation

sidharthv96
Copy link
Member

@sidharthv96 sidharthv96 commented Aug 22, 2023

📑 Summary

Removes the directive grammar from the jison files.
Directives, frontmatter and comments will be preprocessed and removed, before parsing.
Hence, all diagrams will now support directives.

Resolves #4755

📏 Design Decisions

A preprocess step was added, which will extract metadata and remove comments, directives and frontmatter.
wrap and title needed to be handled separately for legacy support.

📋 Tasks

Make sure you

@sidharthv96 sidharthv96 linked an issue Aug 22, 2023 that may be closed by this pull request
@sidharthv96 sidharthv96 changed the base branch from develop to next August 22, 2023 04:50
@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Merging #4759 (667b17b) into develop (abcf2a2) will increase coverage by 34.38%.
Report is 24 commits behind head on develop.
The diff coverage is 64.86%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop    #4759       +/-   ##
============================================
+ Coverage    45.16%   79.55%   +34.38%     
============================================
  Files           53      147       +94     
  Lines         6671    13069     +6398     
  Branches        32      613      +581     
============================================
+ Hits          3013    10397     +7384     
+ Misses        3657     2542     -1115     
- Partials         1      130      +129     
Flag Coverage Δ
e2e 84.72% <82.35%> (?)
unit 43.27% <16.66%> (-1.90%) ⬇️

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

Files Changed Coverage Δ
packages/mermaid/src/diagrams/c4/c4Db.js 65.27% <ø> (ø)
packages/mermaid/src/diagrams/class/classDb.ts 73.65% <ø> (ø)
packages/mermaid/src/diagrams/er/erDb.js 89.28% <ø> (ø)
packages/mermaid/src/diagrams/flowchart/flowDb.js 82.04% <ø> (ø)
packages/mermaid/src/diagrams/gantt/ganttDb.js 78.32% <ø> (+1.72%) ⬆️
packages/mermaid/src/diagrams/git/gitGraphAst.js 56.34% <ø> (ø)
packages/mermaid/src/diagrams/pie/pieDb.ts 100.00% <ø> (ø)
.../mermaid/src/diagrams/quadrant-chart/quadrantDb.ts 100.00% <ø> (ø)
.../mermaid/src/diagrams/requirement/requirementDb.js 98.03% <ø> (ø)
...ckages/mermaid/src/diagrams/sequence/sequenceDb.js 84.64% <ø> (ø)
... and 14 more

... and 119 files with indirect coverage changes

@sidharthv96 sidharthv96 changed the base branch from next to feature/frontmatterConfig August 22, 2023 04:52
@sidharthv96 sidharthv96 changed the base branch from feature/frontmatterConfig to next August 22, 2023 04:53
@sidharthv96 sidharthv96 marked this pull request as ready for review August 22, 2023 06:39
@sidharthv96 sidharthv96 self-assigned this Aug 22, 2023
@netlify
Copy link

netlify bot commented Aug 25, 2023

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 667b17b
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/64f955ec26d4d50007b748f4
😎 Deploy Preview https://deploy-preview-4759--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.

@sidharthv96 sidharthv96 changed the base branch from next to develop August 25, 2023 07:21
* develop: (22 commits)
  docs: Fix link
  Update docs
  fix(pie): align slices and legend orders
  Mermaid version v10.4.0
  unique batches every time, if not repeated tests end up in the same batch
  Added missed .md
  Increase JS heap
  More tests for redirects + prettier
  Fixed redirects inside vitepress, extended tests
  chore: Explain redirect.ts clearly
  Reverted docker compose to develop branch
  Run GA
  Update docs
  Update docs
  fix(er): bug if relationship is declared first
  update latest news
  Removed all n00b file names and added redirects
  test(er): add cypress test on entity name alias
  feat(er): use square brackets to add aliases
  docs(er): add release version for entity name aliases
  ...
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.

Looks good! This makes everything a lot neater!

Some things to do before merging:

  • Does this branch need to be updated now that feat: Support config in frontmatter. #4750 has been merged?
  • Please add /** @deprecated */ TSDoc warnings to and
    _parseDirective: InjectUtils['_parseDirective']
    explaining that the _parseDirective() function is now a no-op, and why it's a no-op (maybe with a link to this PR).
  • Is it worth holding back on merging this until v11? I don't think there are any BREAKING CHANGES (since we've just replaced _parseDirective() with a no-op), so it should be okay, but it's probably worth testing this PR with the @mermaidjs/mermaid-zenuml plugin, just to be on the safe side. Maybe it's also worth renaming this PR to Preprocess directives (e.g. %% {init: }}%%) before parsing diagrams), just to make it a bit more clear what this PR does in the Mermaid v10.x.x release notes.
  • Does this mean that %% {init: ... } %% directives are now supported in sankey diagrams, which previously didn't support these? If yes, it's probably worth documenting this in the PR description.

packages/mermaid/src/Diagram.ts Outdated Show resolved Hide resolved
packages/mermaid/src/preprocess.ts Outdated Show resolved Hide resolved
Copy link
Member

@Yokozuna59 Yokozuna59 left a comment

Choose a reason for hiding this comment

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

This is a partial review, I'll look into remaining files (about 10) in few days since I don't have enough time right now, or you can carry on without my second review :)

@sidharthv96 sidharthv96 changed the title Remove directives from grammar Add directive support to all diagrams by preprocessing Aug 30, 2023
* develop: (31 commits)
  chore: Update docs
  New Mermaid Live Editor for Confluence Cloud (mermaid-js#4814)
  Update link to Discourse theme component (mermaid-js#4811)
  Update flowchart.md (mermaid-js#4810)
  chore: Update docs
  "CSS" instead of "css" in flowchart.md (mermaid-js#4797)
  Update CONTRIBUTING.md
  Update CONTRIBUTING.md
  fix: typos (mermaid-js#4801)
  chore: Align with convention
  add additional test case
  chore(deps): update all patch dependencies
  chore(deps): update all minor dependencies
  added test case
  add sanitize text
  Update docs
  modifications to generic parser
  improvements to parseGenericTypes
  Update packages/mermaid/src/diagrams/class/svgDraw.js
  return comment
  ...
Copy link
Collaborator

@knsv knsv left a comment

Choose a reason for hiding this comment

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

Nice! I hope it was as good for you you as it was to read :)

👍

sidharthv96 and others added 3 commits September 5, 2023 21:57
* develop: (26 commits)
  chore: Fix unit tests
  chore(deps): update all patch dependencies
  Update docs
  chore: remove unneeded `CommomDB`
  fix: Add support for `~test Array~string~`
  chore: Add JSDoc to apply in sequenceDB
  refactor: Tidy up direction handling
  chore: Fix flowchart arrow
  chore: Add test to verify activate
  chore: Update tests snapshot
  fix: mermaid-js#4691 Align arrowheads properly in sequenceDiagram
  chore: move `commonDb` into `diagrams/common/commonDb`
  Update docs
  run prettier fix
  Apply suggestions from code review
  chore: Add comments in edge handling
  chore: Make aggregation arrow transparent
  chore: Remove structuredClone
  chore: Make extension arrow transparent
  chore: Align edge markers properly in class
  ...
@sidharthv96 sidharthv96 mentioned this pull request Sep 6, 2023
4 tasks
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.

Looks good! Thanks for adding those @deprecated warnings in 18079a4.

The only minor potential improvement is to rename and move around some of the tests in the packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js file.


One last thing, this PR adds support for %% {init: {}}%% directives in all diagrams, including diagrams like sankey diagrams that previously didn't support this.

E.g. try rendering the following diagram in the current mermaid.live, and the Mermaid Live preview for this PR:

```mermaid
sankey-beta
  %% changing 'showValues' to true works to modify the diagram
  %%{init: {'sankey': {'showValues': false}}}%%
  %% source,target,value
  Electricity grid,Over generation / exports,104.453
  Electricity grid,Heating and cooling - homes,113.726
```

%%{wrap}%% also previously caused errors in diagrams, but now all diagrams support it (even if it does nothing in most diagrams).

To be honest, I like %% {init: {} }%% being accepted in all diagrams, since it keeps things consistent (even if it's deprecated behavior) 😄. The %%{wrap}%% I'm okay with too, even if it's a no-op in most diagrams. It probably should just be documented in the PR description, so that we know that it's expected behavior.

@knsv knsv enabled auto-merge September 7, 2023 06:01
@knsv knsv added this pull request to the merge queue Sep 7, 2023
Merged via the queue into mermaid-js:develop with commit 07f460a Sep 7, 2023
17 checks passed
fuxingloh referenced this pull request in fuxingloh/contented Oct 3, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [mermaid](https://togithub.com/mermaid-js/mermaid) | [`10.4.0` ->
`10.5.0`](https://renovatebot.com/diffs/npm/mermaid/10.4.0/10.5.0) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/mermaid/10.5.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/mermaid/10.5.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/mermaid/10.4.0/10.5.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/mermaid/10.4.0/10.5.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>mermaid-js/mermaid (mermaid)</summary>

###
[`v10.5.0`](https://togithub.com/mermaid-js/mermaid/releases/tag/v10.5.0):
10.5.0

[Compare
Source](https://togithub.com/mermaid-js/mermaid/compare/v10.4.0...v10.5.0)

#### What's Changed

##### Features

- feat(er): add entity name alias by
[@&#8203;tomperr](https://togithub.com/tomperr) in
[https://github.com/mermaid-js/mermaid/pull/4758](https://togithub.com/mermaid-js/mermaid/pull/4758)

##### Bugfixes

- Fix Twitter fontawesome class in flowchart.md by
[@&#8203;GingerNinjaNicko](https://togithub.com/GingerNinjaNicko) in
[https://github.com/mermaid-js/mermaid/pull/4723](https://togithub.com/mermaid-js/mermaid/pull/4723)
- fix(pie): align slices and legend orders by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[https://github.com/mermaid-js/mermaid/pull/4774](https://togithub.com/mermaid-js/mermaid/pull/4774)
- Update class member handling by
[@&#8203;jgreywolf](https://togithub.com/jgreywolf) in
[https://github.com/mermaid-js/mermaid/pull/4534](https://togithub.com/mermaid-js/mermaid/pull/4534)
- fix(er): allow underscore as leading char by
[@&#8203;tomperr](https://togithub.com/tomperr) in
[https://github.com/mermaid-js/mermaid/pull/4776](https://togithub.com/mermaid-js/mermaid/pull/4776)
- Align arrows on sequence diagram by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4804](https://togithub.com/mermaid-js/mermaid/pull/4804)
- fix: Allow hollow markers on edges by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4788](https://togithub.com/mermaid-js/mermaid/pull/4788)
- fix: Fix for vulnerability making it possible to add javascript in
class names by [@&#8203;knsv](https://togithub.com/knsv)

##### Documentation

- Docs/2910 Remove n00b and fix some docs by
[@&#8203;nirname](https://togithub.com/nirname) in
[https://github.com/mermaid-js/mermaid/pull/4767](https://togithub.com/mermaid-js/mermaid/pull/4767)
- fix: typos by [@&#8203;omahs](https://togithub.com/omahs) in
[https://github.com/mermaid-js/mermaid/pull/4801](https://togithub.com/mermaid-js/mermaid/pull/4801)
- "CSS" instead of "css" in flowchart.md by
[@&#8203;jakeboone02](https://togithub.com/jakeboone02) in
[https://github.com/mermaid-js/mermaid/pull/4797](https://togithub.com/mermaid-js/mermaid/pull/4797)
- fix(docs): Correct repeated text in flowchart.md by
[@&#8203;andriy-koz](https://togithub.com/andriy-koz) in
[https://github.com/mermaid-js/mermaid/pull/4810](https://togithub.com/mermaid-js/mermaid/pull/4810)
- Update link to Discourse theme component by
[@&#8203;gschlager](https://togithub.com/gschlager) in
[https://github.com/mermaid-js/mermaid/pull/4811](https://togithub.com/mermaid-js/mermaid/pull/4811)
- New Mermaid Live Editor for Confluence Cloud by
[@&#8203;zhifeiyue](https://togithub.com/zhifeiyue) in
[https://github.com/mermaid-js/mermaid/pull/4814](https://togithub.com/mermaid-js/mermaid/pull/4814)
- Update classDiagram.md by
[@&#8203;jgreywolf](https://togithub.com/jgreywolf) in
[https://github.com/mermaid-js/mermaid/pull/4781](https://togithub.com/mermaid-js/mermaid/pull/4781)
- Support member definition to initialize class by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4786](https://togithub.com/mermaid-js/mermaid/pull/4786)
- fix: Add support for `~test Array~string~` back in Class by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4805](https://togithub.com/mermaid-js/mermaid/pull/4805)
- Added support for millisecond and second to gantt tickInterval by
[@&#8203;vertxxyz](https://togithub.com/vertxxyz) in
[https://github.com/mermaid-js/mermaid/pull/4778](https://togithub.com/mermaid-js/mermaid/pull/4778)
- Add directive support to all diagrams by preprocessing by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4759](https://togithub.com/mermaid-js/mermaid/pull/4759)
- Update README.md by
[@&#8203;jgreywolf](https://togithub.com/jgreywolf) in
[https://github.com/mermaid-js/mermaid/pull/4780](https://togithub.com/mermaid-js/mermaid/pull/4780)

##### Chores

- chore(deps): update all minor dependencies (minor) by
[@&#8203;renovate](https://togithub.com/renovate) in
[https://github.com/mermaid-js/mermaid/pull/4783](https://togithub.com/mermaid-js/mermaid/pull/4783)
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[https://github.com/mermaid-js/mermaid/pull/4782](https://togithub.com/mermaid-js/mermaid/pull/4782)
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[https://github.com/mermaid-js/mermaid/pull/4809](https://togithub.com/mermaid-js/mermaid/pull/4809)
- chore: move `commonDb` into `diagrams/common/commonDb` by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[https://github.com/mermaid-js/mermaid/pull/4802](https://togithub.com/mermaid-js/mermaid/pull/4802)
- Use utf8 encoding in Jupyter example by
[@&#8203;jonashaag](https://togithub.com/jonashaag) in
[https://github.com/mermaid-js/mermaid/pull/4701](https://togithub.com/mermaid-js/mermaid/pull/4701)
- Update flowchart.md by [@&#8203;Ogglas](https://togithub.com/Ogglas)
in
[https://github.com/mermaid-js/mermaid/pull/4792](https://togithub.com/mermaid-js/mermaid/pull/4792)
- Update flowchart.md by [@&#8203;dsblank](https://togithub.com/dsblank)
in
[https://github.com/mermaid-js/mermaid/pull/4798](https://togithub.com/mermaid-js/mermaid/pull/4798)
- Refactor `cypress/helpers/util.ts` by
[@&#8203;RohanHandore](https://togithub.com/RohanHandore) in
[https://github.com/mermaid-js/mermaid/pull/4340](https://togithub.com/mermaid-js/mermaid/pull/4340)
- refactor: Fix typings in utils.ts by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4826](https://togithub.com/mermaid-js/mermaid/pull/4826)
- Support ClassDefs in external diagrams by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4819](https://togithub.com/mermaid-js/mermaid/pull/4819)
- Fix: flowchartElk Arrow overlap by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4830](https://togithub.com/mermaid-js/mermaid/pull/4830)
- Give markers unique id's per graph by
[@&#8203;chadfawcett](https://togithub.com/chadfawcett) in
[https://github.com/mermaid-js/mermaid/pull/4825](https://togithub.com/mermaid-js/mermaid/pull/4825)

#### New Contributors

- [@&#8203;GingerNinjaNicko](https://togithub.com/GingerNinjaNicko) made
their first contribution in
[https://github.com/mermaid-js/mermaid/pull/4723](https://togithub.com/mermaid-js/mermaid/pull/4723)
- [@&#8203;omahs](https://togithub.com/omahs) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/4801](https://togithub.com/mermaid-js/mermaid/pull/4801)
- [@&#8203;jakeboone02](https://togithub.com/jakeboone02) made their
first contribution in
[https://github.com/mermaid-js/mermaid/pull/4797](https://togithub.com/mermaid-js/mermaid/pull/4797)
- [@&#8203;andriy-koz](https://togithub.com/andriy-koz) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/4810](https://togithub.com/mermaid-js/mermaid/pull/4810)
- [@&#8203;gschlager](https://togithub.com/gschlager) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/4811](https://togithub.com/mermaid-js/mermaid/pull/4811)
- [@&#8203;zhifeiyue](https://togithub.com/zhifeiyue) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/4814](https://togithub.com/mermaid-js/mermaid/pull/4814)
- [@&#8203;vertxxyz](https://togithub.com/vertxxyz) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/4778](https://togithub.com/mermaid-js/mermaid/pull/4778)
- [@&#8203;jonashaag](https://togithub.com/jonashaag) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/4701](https://togithub.com/mermaid-js/mermaid/pull/4701)
- [@&#8203;Ogglas](https://togithub.com/Ogglas) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/4792](https://togithub.com/mermaid-js/mermaid/pull/4792)
- [@&#8203;dsblank](https://togithub.com/dsblank) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/4798](https://togithub.com/mermaid-js/mermaid/pull/4798)
- [@&#8203;RohanHandore](https://togithub.com/RohanHandore) made their
first contribution in
[https://github.com/mermaid-js/mermaid/pull/4340](https://togithub.com/mermaid-js/mermaid/pull/4340)
- [@&#8203;chadfawcett](https://togithub.com/chadfawcett) made their
first contribution in
[https://github.com/mermaid-js/mermaid/pull/4825](https://togithub.com/mermaid-js/mermaid/pull/4825)

**Full Changelog**:
mermaid-js/mermaid@v10.4.0...v10.5.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/levaintech/contented).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4wLjMiLCJ1cGRhdGVkSW5WZXIiOiIzNy4wLjMiLCJ0YXJnZXRCcmFuY2giOiJtYWluIn0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove directives from grammars.
4 participants