[WebProfilerBundle] Inline flowchart-only Mermaid version #54424
Conversation
|
I suggest putting a text file in the repo near the mermaid file mentioning the version of mermaid being included as well as how to build it. Otherwise, updating mermaid will be a pain. |
|
or even better, a script (in a location excluded from the composer archive) that actually generates the file, so that updating mermaid is a matter of updating the version in the script and running it. |
|
@stof are you thinking of something like Intl’s compile script? It’s the only precedent I’m aware of but it comes with the component and require a specific Docker image. |
|
@MatTheCat no, it is not include in the archives: |
|
regarding requiring a Docker image, this script was implemented this way to simplify things as it requires being run with a specific version of icu to achieve its purpose. |
|
Okay, added a Makefile to generate |
| endef | ||
|
|
||
| .PHONY: mermaid-flowchart-v2.min.js | ||
| mermaid-flowchart-v2.min.js: | mermaid-$(if $(tag),$(tag),$(error missing “tag” variable))/node_modules |
There was a problem hiding this comment.
Where is the value of the tag provided ? It looks like it is passed as argument, but it would be better to have the version defined in the script itself so that we track which version is embedded in the repo (instead of having to know which version was used by the last update by asking the dev to remember it)
There was a problem hiding this comment.
The tag would have to be provided when calling make: make tag=v10.9.0 e.g.
Will set its default value to v10.9.0.
There was a problem hiding this comment.
instead of a default value, I would prefer a hardcoded value in the makefile. When wanting to update to a new version, you update the hardcoded version, you run the script, and you commit both changes.
There was a problem hiding this comment.
Forced tag value in cb041e3
| pnpm -C $(@D) install --ignore-scripts | ||
|
|
||
| mermaid-%: | ||
| curl -fL https://github.com/mermaid-js/mermaid/archive/refs/tags/$*.tar.gz | tar -xz --strip-components=1 --one-top-level=$@ |
There was a problem hiding this comment.
this looks like it will create a folder containing the mermaid source code. This should have a gitignore rule to avoid committing it by mistake.
There was a problem hiding this comment.
Okay, was not sure if I was supposed to add a clean rule. Will update .gitignore.
There was a problem hiding this comment.
we can also have a clean command, but that does not remove the need for a gitignore to prevent mistakes.
There was a problem hiding this comment.
Added a clean rule too in de90cc6
|
Thank you @MatTheCat. |
|
Gave a shot at mermaid v11; some changes are needed in the compilation script and the resulting file is > 100 KB heavier. Not sure if a PR is needed 🤔 |
From mermaid-js/mermaid#2920 (comment) there is no official way to generate a diagram-specific script, but it is possible by patching the
packages/mermaid/src/diagram-api/diagram-orchestration.tsfile before building.This PR comes with a Makefile whose default recipe does so, and the
mermaid-flowchart-v2.min.jsfile it generated from v10.9.0.Bumping the script’s version will require to update the Makefile’s
tagvariable to the correspondingmermaid-js/mermaid’s (from https://github.com/mermaid-js/mermaid/tags) and runningmake. The recipe depends on cURL, GNU tar, and pnpm.