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

[MDX] headings prop not being properly populated with nested <Content /> #10447

Closed
1 task
Hugos68 opened this issue Mar 14, 2024 · 4 comments
Closed
1 task
Assignees
Labels
needs discussion Issue needs to be discussed

Comments

@Hugos68
Copy link

Hugos68 commented Mar 14, 2024

Astro Info

Astro                    v4.5.4
Node                     v20.11.0
System                   Windows (x64)
Package Manager          pnpm
Output                   static
Adapter                  none
Integrations             @astrojs/mdx

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

It's easier to explain the issue with a quick example, let's say we have the following:

Layout.astro

<slot />

foo.mdx

## Foo

bar.mdx

---
layout: ./Layout.astro
---

import { Content } from './foo.mdx';

<Content />

## Bar

When rendering the bar.mdx file we get the following:
image

This is great, we now have a reusable markdown file: foo.mdx that we can import and use in multiple places.

Now we want to generate a Table Of Contents for bar.mdx.

Astro's MDX integration has a built in feature that passes a headings prop into the layout that the MDX files are using, this means that Layout.astro will have a headings prop available.

This is what we get back when doing console.log(Astro.props.headings):

[ 
  { 
    depth: 2, 
    slug: 'bar', 
    text: 'Bar' 
  } 
]

This is weird, we are only getting back 1 heading while the page actually has 2 headings.

This happens because the headings prop from Astro is populated through a rehype/remark plugin that only checks for headings in the .mdx/.md file that is being processed, but it does not check for any imported .mdx/md content that could also have headings, for example:

import { Content } from 'foo.mdx';

// This heading is picked up because it is a `node` of type `heading`.
# I am a heading 

// This heading inside `Content` is *not* picked up because it isn't a `node` of type `heading` even though it could still contain markdown headings.
<Content /> 

Docusaurus, a documentation website builder similar to Starlight, had the same issue which can be viewed here: facebook/docusaurus#3915

They ended up solving it by recursively importing and scanning markdown inside their plugin to grab any nested headings, the same solution could be applied here.

I've actually created a proof of concept plugin here which is in a working state but fails to cover some edge cases:
https://github.com/Hugos68/remark-astro-headings

What's the expected result?

When importing .mdx/.md I expect the headings prop to be populated correctly accounting for headings that might be inside imported <Content />.

Link to Minimal Reproducible Example

https://github.com/Hugos68/astro-headings-issue-repro

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Mar 14, 2024
@matthewp
Copy link
Contributor

That solution doesn't work with how Astro works. Astro components compile to a function similar to this:

function App() {
  const props = Astro.props;

  return html`<h1>Title</h1>
    <OtherComponent />
  `
}

I hope that illustrates things. We don't render the component and then hand you props, the props are used to determine what to render. So I don't think we could implement what you're hoping for here.

@matthewp matthewp added the needs discussion Issue needs to be discussed label Mar 15, 2024
@Hugos68
Copy link
Author

Hugos68 commented Mar 15, 2024

Hey @matthewp, first of all, thanks for the quick response! If this is something that cannot be done due to technical limitations on Astro's side then that's unfortunate, I really don't know enough about Astro's internals either to think of any ways to solve this. I did however learn than in the meanwhile I can do:

---
import * as Foo from "./foo.mdx";
import * as Bar from "./bar.mdx";

const merged = Foo.getHeadings().concat(Bar.getHeadings());

console.log(merged);
---

<Foo.Content />
<Bar.Content />

Which results in:

[
  { depth: 2, slug: 'foo', text: 'Foo' },
  { depth: 2, slug: 'bar', text: 'Bar' }
]

It isn't the best solution because you need to manually retrieve and merge headings but it's atleast something that I can use for now. I see you've labeled it as requiring discussion so I'll just wait for that.

@matthewp
Copy link
Contributor

We decided this is a documentation issue and will be updating the docs on MDX.

@matthewp matthewp added the - P2: has workaround Bug, but has workaround (priority) label Mar 15, 2024
@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Mar 15, 2024
@matthewp matthewp removed the - P2: has workaround Bug, but has workaround (priority) label Mar 15, 2024
@matthewp matthewp self-assigned this Mar 15, 2024
@matthewp
Copy link
Contributor

PR is here: withastro/docs#7409

Closing as this is not an Astro bug. Glad you found another way!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Issue needs to be discussed
Projects
None yet
Development

No branches or pull requests

2 participants