remarkjs / react-markdown

Markdown component for React
https://remarkjs.github.io/react-markdown/
MIT License
13.29k stars 876 forks source link

Refactor architecture diagram to use mermaid flowchart syntax #858

Closed cychitivav closed 1 month ago

cychitivav commented 2 months ago

Initial checklist

Description of changes

Refactored the architecture diagram using Mermaid flowchart syntax to improve readability and enhance the visual representation for a clearer understanding of the project structure, improving professionalism despite losing the URLs of links between blocks.

github-actions[bot] commented 2 months ago

Hi! It seems you removed the template which we require. Here are our templates (pick the one you want to use and click *raw* to see its source):

I won’t send you any further notifications about this, but I’ll keep on updating this comment, and hide it when done!

Thanks, — bb

codecov-commenter commented 2 months ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (2245c64) to head (42d5d7c). Report is 19 commits behind head on main.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #858 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 3 3 Lines 1354 1402 +48 Branches 113 114 +1 ========================================= + Hits 1354 1402 +48 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wooorm commented 2 months ago

Looks really tiny to me. I kinda like ASCII art for programming.

cychitivav commented 2 months ago

I like it. Except that the AST types are now no longer links.

Yes, I noticed that, but I couldn't find any information on how to add URLs to the links within the Mermaid diagrams. As an alternative, I thought of preserving the URLs by adding them as a footer or citation in Markdown, but I'm not entirely convinced.

flowchart LR
  markdown[markdown] --> remark[remark]
  subgraph remarkMarkdown[react-markdown]
    direction LR
    remark -- mdast --> remarkPlugins[remark plugins]
    remarkPlugins -- mdast --> remarkRehype[remark-rehype]
    remarkRehype -- hast --> rehypePlugins[rehype plugins]
    rehypePlugins -- hast --> components[components]
  end
  components --> react[react elements]

  click markdown "https://commonmark.org" "Markdown Specification"
  click remark "https://github.com/remarkjs/remark" "Remark Documentation"
  click remarkPlugins "https://github.com/remarkjs/remark/blob/main/doc/plugins.md" "Remark Plugins"
  click remarkRehype "https://github.com/remarkjs/remark-rehype" "Remark Rehype Documentation"
  click rehypePlugins "https://github.com/rehypejs/rehype/blob/main/doc/plugins.md" "Rehype Plugins Documentation"
  click components "#appendix-b-components" "Components Documentation"
wooorm commented 2 months ago

I really have my doubts on this. I do not want to live in a world where ASCII-diagrams are unprofessional. I don’t think all those zoom controls are useful. I think this is fine.

@ChristianMurphy I think you originally made this graph? Thoughts?

remcohaszing commented 2 months ago

I like this, but I do have my doubts considering the readme may be displayed in places that don’t support mermaid.

wooorm commented 2 months ago

@ChristianMurphy Ping :)

ChristianMurphy commented 1 month ago

I think you originally made this graph?

I made this, and based it off the style of the unified architecture diagram https://github.com/unifiedjs/unified?tab=readme-ov-file#overview

I quite like Mermaid as a diagramming tool and don't have any objections to moving this over. It would be kinda nice if the links on the lines could remain, but that's something that may need to be raised upstream at mermaid.

wooorm commented 1 month ago

OK. I will not block mermaid per-se. I personally like ASCII more.

I do think that splitting this graph up into both a graph, and then some links under it, is not an improvement.

If there is a way to fix that, I am 🤷‍♂️ to this PR. If there currently no way to link mdast and hast, then I am :-1: on this PR. I think @remcohaszing too?

@cychitivav Can you somehow link mdast/hast? Report upstream?

remcohaszing commented 1 month ago

I’m also leaning no, but mostly because the Mermaid diagram might not be rendered in places other than GitHub.

wooorm commented 1 month ago

OK, I’ll close this then. I appreciate you contributing, @cychitivav. Thanks, but no thanks!

github-actions[bot] commented 1 month ago

Hi! This was closed. Team: If this was merged, please describe when this is likely to be released. Otherwise, please add one of the no/* labels.