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

chore: update MermaidJS to v10.8.0 #350

Merged

Conversation

oneWaveAdrian
Copy link
Contributor

@oneWaveAdrian oneWaveAdrian commented Feb 22, 2024

Closes #90

Changelog

  • chore: updates MermaidJS to v10.8.0
  • chore: updates yarn.lock to fix expired certificates
  • doc: update Mermaid Version in Readme

- chore: updates MermaidJS
- chore: updates yarn.lock to fix expired certificates
- doc: update Mermaid Version in Readme
@oneWaveAdrian
Copy link
Contributor Author

oneWaveAdrian commented Feb 23, 2024

@shd101wyy Would be great if you could give this a review.

I am not certain what the next steps are to get the VSCode Markdown Preview to support Mermaid 10.8.0 as well, looks like I could open another PR there with the updated crossnote dependency?

Let me know if I can help you out somehow! Really appreciate all the hard work that went into this, thank you!

@oneWaveAdrian
Copy link
Contributor Author

@shd101wyy bump 🙃

@shd101wyy
Copy link
Owner

shd101wyy commented Mar 3, 2024

Hi @oneWaveAdrian , thank you for the PR.

mermaidScript = `<script src="https://${this.notebook.config.jsdelivrCdnHost}/npm/[email protected]/dist/mermaid.min.js"></script>`;
This line might need to be changed to match the 10.8.0 version.

Also, the file located in https://github.com/shd101wyy/crossnote/tree/develop/dependencies/mermaid needs to be manually replaced with the one downloaded from:

wget https://cdn.jsdelivr.net/npm/[email protected]/dist/mermaid.min.js

👍

@shd101wyy shd101wyy self-requested a review March 3, 2024 14:14
Copy link
Owner

@shd101wyy shd101wyy left a comment

Choose a reason for hiding this comment

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

I left a comment above ^^^ Thank you!

I don't know why the CI starts failing 😂

@oneWaveAdrian oneWaveAdrian requested a review from shd101wyy March 3, 2024 22:28
@oneWaveAdrian
Copy link
Contributor Author

@shd101wyy Thanks for the review! Exchanged all files. Additionally fixed linter warnings flagged by CI, shouldn't have failed it though.

Regarding the manual copy task for locally hosted scripts such as mermaid. I wonder if a script running post npm-build that copies the files from node_modules to the respective folder would do the trick? Or else a CI step with auto commit?

Copy link
Owner

@shd101wyy shd101wyy left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thank you for updating mermaid.js and fixing the CI.
It seems like mermaid 10.9.0 just released. I will update it accordingly afterward. That is my bad. I haven't had much time on this project.
Of course, it will be great if we could have a script for automatically downloading all the scripts 👍

@shd101wyy shd101wyy merged commit dc2d9fc into shd101wyy:develop Mar 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update mermaid
2 participants