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

feat: Add SVG diagrams for HTTP sequences #31812

Merged
merged 3 commits into from
Feb 12, 2024
Merged

Conversation

bsmth
Copy link
Member

@bsmth bsmth commented Jan 19, 2024

Description

While we do not yet have support for Mermaid inline in markdown, I've converted a few .png diagrams to SVG using https://www.mermaidchart.com.

Motivation

Removing binaries from the repo, improving maintenance of diagrams, easing transition to mermaid syntax in future.

Additional details

Mermaid syntax
%%{init: { "sequence": { "wrap": true, "width":250, "noteAlign": "center", "messageAlign": "center" }} }%%


sequenceDiagram
    participant Client
    participant Server

    Note left of Client: Request resource
    Client->>Server: GET /doc HTTP/1.1
    Note right of Server: Resource moved<br>Respond with new location
    Server-->>Client: HTTP/1.1 301 Moved Permanently<br/>Location: /doc_new

    Note left of Client: Request resource at new location
    Client->>Server: GET /doc_new HTTP/1.1
    Note right of Server: Return resource
    Server-->>Client: HTTP/1.1 200 OK
Loading
%%{init: { "sequence": { "wrap": true, "width":380, "noteAlign": "center", "messageAlign": "left" }} }%%

sequenceDiagram
    participant Client
    participant Server

    Note over Client: The client signifies its ability to understand two compression algorithms.
    Client->>Server: GET /doc HTTP/1.1<br/>Accept-Encoding: br, gzip
    Note over Server: The resource is sent compressed. The Vary header indicates that content negotiation has been used to select the algorithm.
    Server->>Client: HTTP/1.1 200 OK<br/>Content-Encoding: br<br/>Vary: Accept-Encoding
Loading
%%{init: { "sequence": { "wrap": true, "width": 175, "noteAlign": "center", "messageAlign": "left" }} }%%

sequenceDiagram
    participant Client
    participant Proxy1 as Proxy
    participant Proxy2 as Proxy
    participant Server

    Note over Client: Resource requested.
    Client->>Server: 
    Note over Server: Resource is compressed and returned.
    Server->>Client: 
    Note over Proxy1,Proxy2: Intermediate nodes do not uncompress the body.
    Note over Client: Client decompresses the body.
Loading
%%{init: { "sequence": { "wrap": true, "width": 130, "noteAlign": "center", "messageAlign": "left" }} }%%

sequenceDiagram
    participant Client
    participant N1 as Node
    participant N2 as Node
    participant N3 as Node
    participant Server

    Client-->>N1: Uncompressed
    Note left of Client: Client sends an uncompressed body.
    Note over N1,N3: Intermediate nodes send the body with or without compression on a hop-by-hop basis.
    N1-->>N2: Uncompressed
    N2->>N3: Compressed
    N3-->>Server: Uncompressed
    Note right of Server: The server receives an uncompressed body.

Loading
%%{init: { "sequence": { "wrap": true, "width": 175, "noteAlign": "center" }} }%%

sequenceDiagram
    participant Client
    participant Node1 as Node
    participant Node2 as Node
    participant Server

    Note over Client: Request message
    Client->>Node1: GET /doc HTTP/1.1

    Note over Node1: Shows support for compression while forwarding message.
    Node1->>Node2: GET /doc HTTP/1.1<br/>TE: gzip, br

    Note over Node2: Forwards message
    Node2->>Server: GET /doc HTTP/1.1

    Note over Server: Returns resource in an uncompressed body.
    Server->>Node2: HTTP/1.1 200 OK


    Note over Node2: Compresses body and forwards message.
    Node2->>Node1: HTTP/1.1 200 OK<br/>Transfer-Encoding: br

    Note over Node1: Uncompresses resource and returns message to Client.
    Node1->>Client: HTTP/1.1 200 OK

Loading

Related issues and pull requests

@bsmth bsmth requested review from a team as code owners January 19, 2024 17:09
@bsmth bsmth requested review from teoli2003 and pepelsbey and removed request for a team January 19, 2024 17:09
@github-actions github-actions bot added Content:HTTP HTTP docs system [PR only] Infrastructure and configuration for the project size/xl [PR only] >1000 LoC changed labels Jan 19, 2024
Copy link
Contributor

github-actions bot commented Jan 19, 2024

Preview URLs

(comment last updated: 2024-02-12 11:03:08)

@mdn mdn deleted a comment from Annekalray Jan 19, 2024
@bsmth bsmth added the reduce binary assets Efforts to remove or convert binary assets label Jan 22, 2024
Copy link
Member

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

Can you embed the Mermaid source somewhere, either in the Markdown or in the SVG, as a comment? Makes it easier to make modifications in the future.

@bsmth
Copy link
Member Author

bsmth commented Jan 22, 2024

Can you embed the Mermaid source somewhere, either in the Markdown or in the SVG, as a comment? Makes it easier to make modifications in the future.

Yeah, it's collapsed in the PR description at the moment, but it would be nice to capture it in git. Another option is to put it in https://github.com/mdn/shared-assets/tree/main/images to keep it out of source comments here, what do you think?

edit: Actually in the source is not that bad. We could do:

See the diagram:

<!-- 
sequence.svg Mermaid syntax:

```mermaid
# ...
```
-->

![A sequence diagram](sequence.svg)

@teoli2003
Copy link
Contributor

Another would be to have it in a file next to xyz.svg (xyz.mermaid) that will not be deployed by yari. That way they would be next to each other.

@bsmth
Copy link
Member Author

bsmth commented Jan 23, 2024

Another would be to have it in a file next to xyz.svg (xyz.mermaid) that will not be deployed by yari. That way they would be next to each other.

I also like this suggestion, that's a good one. I'm going to bring it up in content/eng sync so that Yari team are aware.

@hamishwillee
Copy link
Collaborator

hamishwillee commented Jan 26, 2024

Obviously mermaid support in Yari would be the preferred solution - is there an associated yari PR for that?

There is another option - https://mermaid-js.github.io/ provides a mechanism to allow you to click a link to get some text that embeds the image and links to the place where it is edited. The flaw is that it relies on the service existing and running when needed. I.e.

[![](https://mermaid.ink/img/pako:eNqtUk9rgzAU_yrhnTawsl0zKAwrQ0YtW-xNKEFfa6BJXHwZjNLvvlgVJu1hh-YQ8n7_3gvJCSpbI3Do8MujqXCl5MFJ_VIaFlYrHalKtdIQe0vENbhy1uAAB36xXF4AztaZENkm3yWbbV785cPOmaAQwUhptJ4G9uKb-Mn9mX5sU1HssrxgD0-Pc-XY6UZW3-l6lKxI1_Ok3BIy-42ud0SjPI5jpgidDBw1zvpD09e665n_zlpZb2jxfLeJb-fN-r8m7xCBRqelqsODnnptCdSgxhJ4ONa4l_5IJZTmHKS-rcMd01qRdcD38thhBNKTFT-mAk7O4yQaP8UAnn8BbMew9A?type=png)](https://mermaid-js.github.io/mermaid-live-editor/edit#pako:eNqtUk9rgzAU_yrhnTawsl0zKAwrQ0YtW-xNKEFfa6BJXHwZjNLvvlgVJu1hh-YQ8n7_3gvJCSpbI3Do8MujqXCl5MFJ_VIaFlYrHalKtdIQe0vENbhy1uAAB36xXF4AztaZENkm3yWbbV785cPOmaAQwUhptJ4G9uKb-Mn9mX5sU1HssrxgD0-Pc-XY6UZW3-l6lKxI1_Ok3BIy-42ud0SjPI5jpgidDBw1zvpD09e665n_zlpZb2jxfLeJb-fN-r8m7xCBRqelqsODnnptCdSgxhJ4ONa4l_5IJZTmHKS-rcMd01qRdcD38thhBNKTFT-mAk7O4yQaP8UAnn8BbMew9A)

Gives (click on image - the text for this image/link is exported in the actions section)

@bsmth
Copy link
Member Author

bsmth commented Feb 6, 2024

Obviously mermaid support in Yari would be the preferred solution - is there an associated yari PR for that?

Not yet, this might be something to consider for incoming Yari refactoring this year.

There is another option - https://mermaid-js.github.io/ provides a mechanism to allow you to click a link to get some text that embeds the image and links to the place where it is edited.

That's clever. I agree it's tricky to rely on a service like this.
I wonder if this is interesting: https://github.com/mermaid-js/mermaid-cli, i.e.:

npm install -g @mermaid-js/mermaid-cli
mmdc -i input.mmd -o output.svg

We could generate SVGs from .mmd files at some point.

Re: this PR I am happy to constrain it to replacing the .png files with SVG. I have checked everything in here so it's not lost: https://github.com/mdn/shared-assets/tree/main/images/diagrams/http

@Josh-Cena
Copy link
Member

I wonder if this is interesting: https://github.com/mermaid-js/mermaid-cli, i.e.:

npm install -g @mermaid-js/mermaid-cli
mmdc -i input.mmd -o output.svg

FWIW this is what I already do for my mermaid diagrams.

@hamishwillee
Copy link
Collaborator

I would be happy to approve this, but I assume you're still going to add the mermaid text in comments as per #31812 (comment) or as a text file.

Personally I think the right way to do this is to create an issue for the yari support for mermaid (can you/have you done this?). On the assumption that this will happen at some point having the text in the doc content is better.

@github-actions github-actions bot added size/l [PR only] 501-1000 LoC changed and removed size/xl [PR only] >1000 LoC changed labels Feb 12, 2024
@bsmth
Copy link
Member Author

bsmth commented Feb 12, 2024

I would be happy to approve this, but I assume you're still going to add the mermaid text in comments as per #31812 (comment) or as a text file.

Done in 4236fe9

Personally I think the right way to do this is to create an issue for the yari support for mermaid (can you/have you done this?). On the assumption that this will happen at some point having the text in the doc content is better.

Sure, I've opened this one which might be a good tracker -> mdn/yari#10497

Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Looks great to me. Thanks for this.

@hamishwillee hamishwillee merged commit f2f16cd into mdn:main Feb 12, 2024
9 checks passed
@bsmth bsmth deleted the http-diagrams branch February 12, 2024 21:05
@bsmth
Copy link
Member Author

bsmth commented Feb 12, 2024

Thanks, all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTTP HTTP docs reduce binary assets Efforts to remove or convert binary assets size/l [PR only] 501-1000 LoC changed system [PR only] Infrastructure and configuration for the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants