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

Change of Remark plugins order for MDX integration after Astro 2.0 upgrade #6079

Closed
1 task done
njakob opened this issue Jan 31, 2023 · 2 comments
Closed
1 task done

Comments

@njakob
Copy link
Sponsor

njakob commented Jan 31, 2023

What version of astro are you using?

2.0.4

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Linux

Describe the Bug

While migrating to Astro 2.0 the "reading time" feature for my articles based on the Astro documentation (https://docs.astro.build/en/guides/markdown-content/#example-calculate-reading-time), I'm getting different reading times.

This happens as the Remark plugin is getting already "preprocessed" nodes by the built-in highlighter (Shiki in my case). In other words, toString(tree) already includes some HTML which therefore adds a lot more plain text to the reading-time function call.

While digging into MDX, I realized that the merging of Markdown plugins got changed by #5684 (especially here). Before, Markdown plugins were added before any MDX or built-in syntax highlighters and extensions.

Since it seems pretty common to add plugins and their order is quite relevant, we should most likely only inject highlighters as the last plugins. Unless this is due to another reason?

Besides, in the PR mentions that we could eject from the highlighters and define the order on the consumer side. However, since remark-shiki is not directly exposed and cannot be directly imported, the code would need to be copy-pasted. Therefore, we could also export these "internal" plugins.

You can observe the change in behavior through the following reproducible setup and change back to the following package.json dependencies:

"dependencies": {
  "@astrojs/mdx": "^0.14.0",
  "astro": "^1.9.2",
  "mdast-util-to-string": "^3.1.1",
  "reading-time": "^1.5.0"
}

Happy to hear your thoughts here!

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-vcjigg?file=astro.config.mjs

Participation

  • I am willing to submit a pull request for this issue.
@bholmesdev
Copy link
Contributor

@njakob Good catch! Yes, this was definitely a regression. We caught a similar issue migrating docs.astro.build to 2.0 as well. Merging a fix shortly!

@njakob
Copy link
Sponsor Author

njakob commented Feb 1, 2023

That's amazing, thank you for picking this up @bholmesdev!

Looking at the extra plugins used in the documentation, having the same issue make sense...

I would definitely test this as soon as the fix gets released 🙌

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

No branches or pull requests

3 participants