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

Fix MDX heading IDs generation when using a frontmatter reference #5978

Merged
merged 2 commits into from
Jan 26, 2023

Conversation

HiDeoo
Copy link
Member

@HiDeoo HiDeoo commented Jan 25, 2023

Changes

This PR fixes #5780.

When using a frontmatter reference in a MDX heading, the ID generated for the heading is not using the reference value. For example,

---
title: The Title
---

# {frontmatter.title}

would generate <h1 id="frontmattertitle">The Title</h1> instead of <h1 id="the-title">The Title</h1>.

This pull request updates the @astrojs/markdown-remark rehype plugin generating these IDs to fix this behavior:

  1. For MDX document, if the heading is a MDXTextExpression, e.g. # {frontmatter.title}, the plugin will use the associated ESTree entry to get the variable path, e.g. ['frontmatter', 'title'].
  2. If the variable is referencing a frontmatter value, the plugin try to get the value from the Astro data containing the frontmatter.
  3. If the value is found, the plugin will use it to generate the heading ID.

Note: the added dev dependencies to @astrojs/markdown-remark are only used to reference types.

Testing

I added a new fixtures test-with-frontmatter.mdx to the @astrojs/mdx test suite to test the changes.

The tests covers various cases and syntaxes from a basic variable to nested list items:

---
title: The Frontmatter Title
keywords: [Keyword 1, Keyword 2, Keyword 3]
tags:
  - Tag 1
  - Tag 2
  - Tag 3
items:
  - value: Item 1
  - value: Item 2
  - value: Item 3
nested_items:
  nested:
    - value: Nested Item 1
    - value: Nested Item 2
    - value: Nested Item 3
---

# {frontmatter.title}
## frontmatter.title
### {frontmatter.keywords[1]}
### {frontmatter.tags[0]}
#### {frontmatter.items[1].value}
##### {frontmatter.nested_items.nested[2].value}
###### {frontmatter.unknown}

Docs

I could not find any specific mention regarding this specific behavior in the docs and I'm not sure if it's worth mentioning it.

@changeset-bot
Copy link

changeset-bot bot commented Jan 25, 2023

🦋 Changeset detected

Latest commit: 05ec52f

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: integration Related to any renderer integration (scope) feat: markdown Related to Markdown (scope) labels Jan 25, 2023
Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Changes look good to me but want to pull @bholmesdev in since I know he's been working with MDX a lot lately

Copy link
Contributor

@bholmesdev bholmesdev left a comment

Choose a reason for hiding this comment

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

A few nits, but amazing work. Thorough checks and unit testing here. An IDeal fix 👏

packages/markdown/remark/src/rehype-collect-headings.ts Outdated Show resolved Hide resolved
packages/markdown/remark/src/rehype-collect-headings.ts Outdated Show resolved Hide resolved
@bholmesdev bholmesdev merged commit 7abb1e9 into withastro:main Jan 26, 2023
@astrobot-houston astrobot-houston mentioned this pull request Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: markdown Related to Markdown (scope) pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generated heading id from MDX is not expanded
3 participants