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

✨ Adds mermaidjs as node package #242

Closed
wants to merge 14 commits into from
Closed

✨ Adds mermaidjs as node package #242

wants to merge 14 commits into from

Conversation

OCram85
Copy link
Contributor

@OCram85 OCram85 commented Apr 19, 2021

📦 Content

🔖 Refs

OCram85 and others added 5 commits April 19, 2021 19:19
* replaces possible backslash for win environments

like in hugo docs explained:  `.File.Path` returns paths joined by backslashes instead of slashes.

* Updating the README!

* Updating the README!

* Updating the README!

* Updating the README!

* Updating the README!

* Updating the README!

* Updating the README!

* Updating the README!

* Updating the README!

* Updating the README!

* Updating the README!

* fix mixture of permanent and relative links

* Updating the README!

* Updating the README!

* Replace highlightBlock with highlightElement

* Updating the README!

* Updating the README!

* deps: bump versions to latest

* Updating the README!

* Updating the README!

Co-authored-by: Henk Verlinde <henk@henkverlinde.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Mike Pianka <47948499+mikepianka@users.noreply.github.com>
Co-authored-by: umatare5 <umatare5@gmail.com>
Co-authored-by: Henk Verlinde <henk@ventizo.com>
@OCram85 OCram85 changed the title 🐛 Adds mermaidjs as node package Adds mermaidjs as node package Apr 19, 2021
@h-enk h-enk self-requested a review April 19, 2021 19:19
@OCram85
Copy link
Contributor Author

OCram85 commented Apr 20, 2021

@h-enk : Sorry for the changes in the higlight.js and img shortcode. Maybe my diff wasn't on top of the latest master branch commit.

@OCram85 OCram85 changed the title Adds mermaidjs as node package ✨ Adds mermaidjs as node package Apr 20, 2021
@h-enk
Copy link
Member

h-enk commented Apr 20, 2021

@OCram85, nice work!

Mermaid applies (a lot of) inline styles. Currently these are blocked by the Content Security Policy. See also: https://deploy-preview-242--doks.netlify.app/docs/examples/mermaid/

It looks like there's no solution yet to remove the inline styles by config — see also mermaid-js/mermaid#856 + mermaid-js/mermaid#1054

I would like to avoid script-src: 'self' 'unsafe-inline', but I don't see a smart trick adding (that much) auto-generated hashes?

P.S. Your package-lock.json is still out of sync

@h-enk
Copy link
Member

h-enk commented Apr 20, 2021

Ah, I need more coffee 😉

I mean style-src 'self' 'unsafe-inline' — and I can live with that

@OCram85
Copy link
Contributor Author

OCram85 commented Apr 20, 2021

Oh I really couldn't test mermaid output in netlify deployments.
I'm also looking for a solution for the build in styles/ themes. I really would be awesome to be able to match the doks color scheme with the mermaid theme. But I couldn't find simple solution without doubling the sccs variables in the mermaid js theme definition ^^

@h-enk
Copy link
Member

h-enk commented Apr 20, 2021

Oh I really couldn't test mermaid output in netlify deployments.

Yeah Netlify's deploy preview is super handy for that: https://deploy-preview-242--doks.netlify.app/

@OCram85
Copy link
Contributor Author

OCram85 commented Apr 20, 2021

I'm by far not an expert in this. I have to grab another coffee and dig into this, too. 🐰

@OCram85
Copy link
Contributor Author

OCram85 commented Apr 22, 2021

@h-enk: I really don't get it with the package-lock.json. I just restored the package files from master and added mermaid via npm install -D mermaid. But it sill leads into the merge conflicts 😢

@h-enk
Copy link
Member

h-enk commented Apr 22, 2021

@h-enk: I really don't get it with the package-lock.json. I just restored the package files from master and added mermaid via npm install -D mermaid. But it sill leads into the merge conflicts 😢

@OCram85, did you then run npm install --package-lock-only? See also: Resolving lockfile conflicts

@OCram85
Copy link
Contributor Author

OCram85 commented Apr 23, 2021

Nope, unfortunately no changes with --package-lock-only

@h-enk
Copy link
Member

h-enk commented Apr 23, 2021

Nope, unfortunately no changes with --package-lock-only

Ohh, very very funny gif!!! You got me laughing out loud.

Good, then we'll go for the more blunt way: delete the ./node_modules folder and .package-lock.json, and then run npm i.

@OCram85
Copy link
Contributor Author

OCram85 commented Apr 23, 2021

That's what I already did with the latest commits:

  1. git clean --hard
  2. fetch upstream/master
  3. checkout package.json and package-lock.json
  4. deleted node_modules
  5. npm install --package-lock-only
    • tested with and w/o flag --package-lock-only
  6. npm install -D mermaid
    • tested with and w/o flag --package-lock-only
  7. added changes to repo
    • git add -A && git commit -m 'adds mermaid'
  • Could this depend on the current npm or windows environment ?
  • I could remove mermaid from this PR and you could add memaid into he package.json yourself?

@h-enk
Copy link
Member

h-enk commented Apr 24, 2021

Hmm, it looks more like a npm thing to me, but I'm no expert. I'm using npm v7.5.3 (with Node v15.9.0) on Windows 10 Pro 20H2.

Let's see what happens if you remove Mermaid /make your package.json changes undone. I've now added Mermaid: 6d5ec50

@h-enk h-enk removed their request for review April 24, 2021 08:53
@OCram85
Copy link
Contributor Author

OCram85 commented Apr 26, 2021

I'm running this environment:

# Windows 10 Version 1909
# node: v10.16.0
# npm: 6.9.0

... I'm updating my node/ npm and will try it again.

Is there any need to include the package-lock.json ?
I always thought the package.json defines the hard dependencies and the package-lock resolves just local the soft/nested packages.

@OCram85
Copy link
Contributor Author

OCram85 commented Apr 28, 2021

Finally fixed the lint issues :D 🤖 🚀 🛸

@h-enk
Copy link
Member

h-enk commented Jul 2, 2021

@OCram85, super thanks for your efforts. However, to speed things up, I'm going to close your PR and go forward w/ #369

@h-enk h-enk closed this Jul 2, 2021
@OCram85 OCram85 deleted the mermaidjs branch July 20, 2021 06:21
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.

2 participants