eecs485staff / primer-spec

A Jekyll theme for sites with content-heavy pages
https://eecs485staff.github.io/primer-spec/
MIT License
22 stars 13 forks source link

Pin to Mermaid v9 #254

Closed seshrs closed 1 year ago

seshrs commented 1 year ago

Context

Closes eecs280staff/p5-ml#183.

@amirkamil noticed that Mermaid diagrams weren't rendering on the P5 spec:

Expected Actual
Screenshot 2023-11-06 at 8 15 52 PM Screenshot 2023-11-06 at 8 14 11 PM

Digging a bit, I found https://github.com/mermaid-js/mermaid/issues/4442, which noted that the root cause was a breaking change in v10 of MermaidJS:

Mermaid is ESM only!

I didn't notice when adding MermaidJS support to Primer Spec, but it turned out that the documentation had let me to always use the latest version of MermaidJS instead of pinning to a specific version — hence, we were exposed to the breaking change :(

In this PR

This commit changes the Primer Spec to always use MermaidJS v9, which is compatible with CJS and exposes the global variable mermaid.

In future, it would make sense to follow-up how best to upgrade to Mermaid v10. But this works for now.

Validation

Visit the preview URL's "Diagrams" demo. Notice that the Mermaid diagrams now render again.

github-actions[bot] commented 1 year ago

The spec from this PR is available at https://preview.sesh.rs/previews/eecs485staff/primer-spec/254/.

(Available until Fri Dec 08 2023.)

awdeorio commented 1 year ago

In future, it would make sense to follow-up how best to upgrade to Mermaid v10. But this works for now.

If you feel this is something that's important to track, consider making an issue :)