DanielFlaum / grav-plugin-mermaid-diagrams

Mermaid Diagrams is a Grav plugin that adds simple and powerful diagrams functionality
https://github.com/DanielFlaum/grav-plugin-mermaid-diagrams
MIT License
12 stars 1 forks source link

Diagrams fail to render in the browser when Grav's JS minification is enabled #2

Open awrog opened 4 years ago

awrog commented 4 years ago

Great plugin! it works like a charm in a clean GRAV site. In an existing site (Learn2 theme) with more active plugins I get text instead of a diagram. I cannot figure out which plugin is causing the problem.

Screenshot - 8_1_2019 , 13_40_02

Settings of the page: Screenshot - 8_1_2019 , 13_42_35

awrog commented 4 years ago

Installed a fresh Learn2 GRAV (Skeleton) site. Disabled all plugins Installed the mermaid plugin. No diagram, only the code.

DanielFlaum commented 4 years ago

I rolled out a fresh Learn2 skeleton myself and successfully reproduced the issue, even without disabling other plugins.

The Mermaid Diagrams plugin does work on another, non-vanilla site of mine, but I made lots of changes to Grav's default configuration on that one. So I'm going to explore how changing Grav's settings, especially to do with CSS and JS pipelines, may be playing a role here.

DanielFlaum commented 4 years ago

Found it! If Grav's JavaScript pipeline is enabled and configured to perform minification, then Mermaid Diagrams goes to pieces. The minify procedure performs some kind of transformation that breaks things.

@awrog You can work around this issue for now by disabling minification in the JavaScript pipeline. If using the Admin Panel, go to Configuration > System > Assets and you'll find the switch for it. If you'd rather edit system.yaml, you're looking for assets: ✅✅js_minify:.

With the fresh Learn2 skeleton and the Mermaid Diagrams plugin, disabling minification added just 2 kilobytes to the page weight. The pipeline still unifies all the JavaScript, so your server won't be hit by any extra HTTP requests. That makes it a very small performance hit overall, but you'll want to double check the effect it has on sites with more plugins or a different theme.

DanielFlaum commented 4 years ago

For my future reference:

Grav's central logic for the JS pipeline is in public function renderJs($assets, $group, $attributes = []) in /system/src/Grav/Common/Assets/Pipeline.php. Grav uses matthiasmullie/minify for both CSS and JS minification.

awrog commented 4 years ago

You rock, it works! The same applies for the plugin Minify Html

DanielFlaum commented 4 years ago

No problem, glad I could help!

I'm gonna keep this issue open, since a proper solution is for me to get this working with Grav's pipeline.

leycec commented 4 years ago

Thanks for reopening this, @DanielFlaum! You rock the... mermaid rocks?

Until resolved, it'd be awesome if the next stable plugin release could push an interim notification warning the site maintainer about this – perhaps via a Dashboard notification or explicit Report in the Admin Panel or even just a log message. At the moment, only we obsessive-compulsive few who assiduously check GitHub issues for installed plugins have been apprised of the breakage. we cray-cray, yo

Phenomenal fork, JavaScript minification notwithstanding. Thanks for all the diagrammatic visualization for now and hopefully all eternity.

DanielFlaum commented 4 years ago

Hey @leycec, thanks for the kudos and the suggestion! I can set up a notification no problem. I am (and remain) a Grav beginner, and did not think to go looking for those things.

Since you found this issue, would I be correct in assuming that you were wanting to use JS minification? Since this issue has a workaround that the original submitter found acceptable, I've been leaving it hanging until someone explicitly asked for it to be solved.

If minification is something you need, let me know, and I can get started on trying to fix it. It may take me a few weeks, depending on my schedule, but I don't expect it to be a rabbit hole.

Cheers!