Markdown headings generate trailing slash - can it be trimmed?

JonnyBurger commented 1 year ago

Have you read the Contributing Guidelines on issues?

[X] I have read the Contributing Guidelines on issues.

Prerequisites

[X] I'm using the latest version of Docusaurus.
[X] I have tried the npm run clear or yarn clear command.
[X] I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
[X] I have tried creating a repro with https://new.docusaurus.io.
[X] I have read the console error message carefully (if applicable).

Description

The following Markdown:

## My heading <span/>

Generates a link <a href="#my-heading-"/>.

It would be nice if the trailing slash is trimmed. Background: We want to add <AvailableFrom version="4.0.0"/> badges to our headings, but it makes the links ugly.

Reproducible demo

https://stackblitz.com/edit/github-qfmbcz?file=docs%2Fintro.md

Steps to reproduce

Open repro

Expected behavior

No trailing -

Actual behavior

Trailing -

Your environment

Public source code:
Public site URL:
Docusaurus version used:
Environment name and version (e.g. Chrome 89, Node.js 16.4):
Operating system and version (e.g. Ubuntu 20.04.2 LTS):

Self-service

[X] I'd be willing to fix this bug myself.

Josh-Cena commented 1 year ago

Agreed that trimming trailing - will look better. However, do we want to deviate from GitHub Markdown? I'm not sure.

slorber commented 1 year ago

However, do we want to deviate from GitHub Markdown? I'm not sure.

I don't think GitHub / GFM computes any slug for headings so there's no real inspiration from GitHub to take here, apart that we already use the github-slugger npm package.

A workaround is to use explicit heading ids to override the default generated heading id.

In general, I wonder if there's not a better solution we could propose instead of using this:

## My heading <AvailableFrom version="4.0.0"/>

With MDX 2 the recommended way to declare explicit heading ids will be:

## My heading <AvailableFrom version="4.0.0"/> {/* headingId */}

@wooorm was wondering if you know any solution to declare multiple metadata for a single heading?

I assume it's preferable to keep using Markdown heading syntax for tooling compatibility, but wonder if there's a kind of the equivalent of directives for headings?

I'm thinking of something like this:

## My heading {id="my-heading", from="4.0.0", new=true}

Josh-Cena commented 1 year ago

I don't think GitHub / GFM computes any slug for headings so there's no real inspiration from GitHub to take here, apart that we already use the github-slugger npm package.

Standalone Markdown files do have anchors generated for each heading, e.g. https://github.com/facebook/docusaurus#code-of-conduct

wooorm commented 1 year ago

GitHub

GitHub definitely does compute slugs! e.g., https://github.com/facebook/docusaurus#installation (as Josh points out right now)

Standalone Markdown files

That’s the point for sticking with the common system: a) so that the slug is the same for the file on GH as for rendered on your site b) so that the slug is the same when working in VS code with autocompletion c) etc

equivalent of directives

Directives are generic, you can map them to headings. ::Heading[*whatever*]{#id}. Which is equivalent to <Heading id="id">*whatever*</Heading>

slorber commented 1 year ago

Ah, thanks, indeed repo README.md files have heading ids!

However, according to my tests headings seems to be slugified after being rendered and do not consider the markup as part of the heading: ## Installation xyz => #installation-xyz.

Wonder what's the behavior of rehypejs/rehype-slug for the case above, does it do exactly like GitHub? (not super fan of our current implementation of our TOC/headingId so open to change it if there's a better way)

Directives are generic, you can map them to headings. ::Heading[whatever]{#id}. Which is equivalent to whatever

Yes, I understand that, but in practice that seems not very convenient if IDEs, GitHub and translation tools do not natively understand this directive as a heading 😅

I'd really prefer to keep using ## for level2 headings, and still be able to assign custom metadata.

Maybe an MDX comment can work although a bit verbose

## My heading {/* {id="my-heading", from="4.0.0", new=true} */}

slorber commented 1 year ago

That’s the point for sticking with the common system: a) so that the slug is the same for the file on GH as for rendered on your site b) so that the slug is the same when working in VS code with autocompletion c) etc

Yes definitively the goal here, just wondering what's the solution to have this:

## My heading <AvailableFrom version="4.0.0"/>

And have all those properties above. If this can render well on GitHub UI that's also great 😄 .

JonnyBurger commented 1 year ago

Thanks for tracking down the source!

Will Docusaurus consider making an option? trimTrailingDashes that is opt-in? If so, I'd consider making the PR.

wooorm commented 1 year ago

after being rendered

To add some nuance: GitHub generates slugs on an HTML AST, based on “sort of” the textContent. That means if doesn’t see markdown escapes, ** for strong, or HTML tags, but does see “inside” escapes/strong/elements.

But, it doesn’t do that on the actual rendered DOM. That doesn’t matter for GitHub. But it does matter for MDX, which has components, that need to be evaluated, which would render to something else. That is to say, the HTML AST would see Installation xyz for ## Installation xyz, but can’t see that for ## Installation <SomeComponent /> even though a human might see it on screen.

Wonder what's the behavior of rehypejs/rehype-slug for the case above, does it do exactly like GitHub?

Yes, it matches GitHub. But it can’t do things with evaluated components from MDX or so.

in practice that seems not very convenient if IDEs, GitHub and translation tools do not natively understand this directive as a heading

And that’s exactly the reason not to mess with slugs: other tools don’t understand your new slug syntax.

I would recommend, in MDX:

<MyFancyHeadingWrapper id="my-heading" from="4.0.0" new>
  ## My heading
</MyFancyHeadingWrapper>

Or, in markdown:

<a name="my-heading"></a>
## My heading

slorber commented 1 year ago

Thanks for all those details @wooorm, <MyFancyHeadingWrapper> looks like a great solution.

We will inspect our code and look at what we do wrong. I don't think we align 100% with how GitHub generates slugs

Will Docusaurus consider making an option? trimTrailingDashes that is opt-in?

I don't think it's necessary @JonnyBurger, it looks like we are doing something wrong by default so we'd rather fix that in the first place. In the meantime, you can use an explicit id.

wooorm commented 1 year ago

Why do you think you are doing something wrong? GH too turns ## My heading  -> my-heading-?

JonnyBurger commented 1 year ago

Thanks a lot for all these tips!

I agree, it doesn't make sense to deviate unnecessarily from the GitHub flavor. I didn't think of just putting a tag <a id="my-heading" />, this is a pretty good solution, since I can also put it inside the <AvailableFrom> component I was planning to use.

From my side, the issue is clear, unless you want to do some changes nonetheless. I'll be fine if you decide to close it.

slorber commented 1 year ago

Why do you think you are doing something wrong? GH too turns ## My heading  -> my-heading-?

Ah you are right, I thought we had a different behavior but GH also has leading/trailing dashes for

## 🚀 New Feature

## My heading <span>

Let's close it then, it's probably better to do nothing if we are already in sync with GitHub

facebook / docusaurus