Jellyvision / jsdoc-mermaid

A tool to automagically create flowcharts and diagrams in your jsdocs.
MIT License
56 stars 12 forks source link

Add support for Mermaid configuration options, version and style #5

Open cmidgley opened 4 years ago

cmidgley commented 4 years ago

This pull has the following features:

As an aside, the prior Mermaid version used had a sizing problem with classDiagram (massive rendering), but the latest (8.5.0) messes up graph (very tall boxes around boxes). If you set the flowchart option "htmlLabels": false this problem goes away. This is shown in the example configuration in the README, but hopefully this problem will be fixed in a future Mermaid release.

cmidgley commented 4 years ago

I made the changes you recommended and also merged your refactored code. I attempted to match your style for the changes, but wasn't totally clear on the best way to handle the fact that you were using templates that won't work (at least not as global constants) with these changes. My thought was to still use the template syntax (${xxx}) but use a string and .replace to make the change. If this isn't what you think appropriate just let me know.

Thanks for being so responsive!

cmidgley commented 4 years ago

I added one more feature called disableScript which tells it to not emit the code to load or initialize Mermaid. It still does the <div> tag. This is so that you can use this when including a template that already has Mermaid code. Turns out I needed this as my JSDoc tutorial files also needed Mermaid, so I placed the Mermaid init/startup script in my template, thereby requiring this feature.

patrickdbakke commented 4 years ago

@cmidgley 👍 thanks for making the changes. Since it's such a small project, I'm not sweating the linting details. What you got is great.

I'm working to get someone with our npm package ownership to release these changes. :D

cmidgley commented 4 years ago

Hi - I realized that JSDoc has a serious limitation in how @tags are managed, in that you can't retain the context of the tag. As I'm sure you know, this means that all the Mermaid diagrams end up at the bottom of the comments, rather than being inline where they are referenced. I have decided to take a different approach, where I use markdown instead with the mermaid language selector, and then use a tiny javascript client-side method that adds a mermaid class to the parent tag and this works great, with inline context. This means I will no longer be using this module. Feel free to accept or reject whatever of these changes you want.

Also - I found another small issue. If the mermaid section is in a classdesc section (rather than description section, such as when you use both a class and a constructor) the code doesn't work. It looks like a simple fix, but as I said, it's not the path I am taking any more.

Hoppi164 commented 11 months ago

Hi there.

I think this change is a great improvement to the package; but i'd like to suggest one modification. We should use mermaid.run or mermaid.init to initialize mermaid after all the page has fully loaded.

This will fix several of the problems described in issue: #9 I experienced some of these issues ( unable to render erDiagrams ) I've narrowed down the issue to being a race condition of the mermaid script starting too early. The Mermaid docs advise to use startOnLoad: false and then to mermaid.run to instruct mermaid to start AFTER the full page has loaded.

As of mermaid v10 you should

mermaid.initialize({ startOnLoad: false });
await mermaid.run();

Before mermaid v10 you should

mermaid.initialize({ startOnLoad: false });
await mermaid.init();

https://github.com/mermaid-js/mermaid/blob/develop/docs/config/usage.md#using-mermaidrun

Alternatively I could add this change in a separate PR, however it will depend on this one being merged first, as it'll involve passing data through using your MERMAID_CONFIG

andresdiaz29 commented 1 month ago

Why is this not approve it yet? I really need this plugin with the latest version of Mermaid :(

Hoppi164 commented 1 month ago

Why is this not approve it yet? I really need this plugin with the latest version of Mermaid :(

The last commit on this repo was 4 years ago.

I've also tried getting this pr merged, but i don't think this package is regularly maintained anymore, unfortunately.

andresdiaz29 commented 1 month ago

Too sad... Maybe if you create another package and publish it, you can get the reference from Mermaid official webpage! Do you wanna try? I can be a contributor! @Hoppi164

patrickdbakke commented 1 month ago

Hey @Hoppi164 @andresdiaz29 . Apologies for the delay; thanks for the recent comments. I am no longer employed by jellyvision, so i'm not sure what level of maintenance priority this package gets internally there.

Fortunately, AFAICT, I am still a project owner. I will review the changes and approve + merge + publish a new version later tonight.

Hoppi164 commented 1 month ago

@patrickdbakke you absolute legend!

patrickdbakke commented 1 month ago

@cmidgley @Hoppi164 nice work on the changes here. I left some feedback on a few minor points. Let me know your thoughts on these comments. I'm ready to merge and release pending your feedback on those!

@cmidgley If you're swamped, i can make a commit on your behalf with the requested changes, but it'd slightly misattribute in the git blame. I wanna make sure you get credit. LMK your thoughts!

andresdiaz29 commented 1 month ago

Should we add an extra validation here? Just in case

if (typeof mermaid.run === 'function') {
  await mermaid.run();
} else if (typeof mermaid.init === 'function') {
  mermaid.init();
} else {
  throw new Error('Unsupported library version');
}
Hoppi164 commented 1 month ago

Should we add an extra validation here? Just in case

Yeah that looks good to me

patrickdbakke commented 1 month ago

@cmidgley If you're swamped, i can make a commit on your behalf with the requested changes, but it'd slightly misattribute in the git blame. I wanna make sure you get credit. LMK your thoughts!

Acknowledging some folks are waiting on these changes - if i don't hear back from @cmidgley by tomorrow, i'll make the requested changes and merge.

cmidgley commented 1 month ago

Hi everyone - I'm not using this package these days, so if somebody else wants to take the ball and run with it, then by all means go for it. Don't worry about attributions, get what you need done in the easiest way possible!