Add "allowHTML" option for Markdoc with HTML parsing/processing #7597
Conversation
This uses htmlparser2 to perform a pure token transform/mutation on the markdown-it tokens, replacing the original raw HTML string tokens with a richer set of tokens per HTML node, and in the process Markdoc tags are interleaved in the resulting token graph at the appropriate locations This removes the legacy config of the @astrojs/markdoc integration entirely (suggested by @bholmesdev) and introduces a new type for options to be specified in the astro config, initially, with just the new "enableHTML" option When "enableHTML" is *not* enabled (the default), the behavior of the entire @astrojs/markdoc integration should remain functionally equivalent to before this change
also: * cleaned up " to ' for astro project preferred linting * made the html rendering test fixture use a dynamic path
… definition for html
🦋 Changeset detectedLatest commit: af54e3c 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
bot
added
the
pkg: integration
|
I had to create a new branch with proper author commit info for myself (sorry had a mix bag of my work email in the original commit set). This branch/PR is the exact same commit history with fixed author info. See #7582 for previous discussion |
|
Seems like the failure was probably a transient issue with the CI runner, not this branch/PR |
|
@alex-sherwin Checking back in on this, my apologies! Were the comments from the previous PR addressed here? |
Hi @bholmesdev Yes, all tasks/conversations from the previous PR were addressed (the fixes for them were part of the commit stream in the previous PR #7582, so you can refer there if you want to see how I addressed them) Although it is the exact same commits (with fixed author info) in this PR, as well |
There was a problem hiding this comment.
Fantastic work! No complaints on the code. From here, we'll need to add a changeset and some documentation before it's merge-ready. You can:
- Run
pnpm changesetfrom the repo root to add a description of the changes. This can be brief, and may include a code block describing the config you need to add. - Add a markdoc README heading for allowing HTML. Maybe a new h2 below the "Markdoc config" heading?
Regarding this... all the headings in that part of the README is about the markdoc.config.js which is just re-explaining the Markdoc lib's config object and how it can integrate with Astro Currently there's no docs about configuring the integration itself (because before this PR there is no supported integration options), @bholmesdev, do you think it should be similar to https://docs.astro.build/en/guides/integrations-guide/tailwind/#configuring-the-integration where it's just a "Configuring the Integration" heading as a sibling to all the existing headings (which, again, are all markdoc.config.js specific) |
|
@alex-sherwin That's a good point. I'll admit Markdoc's docs are more unique in how they read like a "guide" instead of a list of config options. Maybe we can change the "Configuration" header to "Markdoc config" Then, at the end of the section (just about Examples), we can add a new section called "Integration config options" with |
|
@bholmesdev I think this should be all set, but I'm not sure the CI checks cancelled themselves |
There was a problem hiding this comment.
Some comments on readability. Handing off to @sarah11918 and the docs people for review
There was a problem hiding this comment.
Thank you for this contribution, @alex-sherwin! You... might make me actually want to use Markdoc? 😄
Just left some finer docs editing for your consideration on top of Ben's comments, which I whole-heartedly approve!
Leaving final approval to @bholmesdev!
@sarah11918 I don't see why anyone wouldn't use Markdoc over Markdown since it now exists... I just see it as superset of Markdown (so no downsides) while giving you opt-in extensibility for rich content provided as data Win-win for everyone.. essentially no long-term maintenance burdens and baggage like MDX brings to the table (I view MDX as turning your docs into code, while Markdoc remains data but enables rich rendering capabilities) |
|
Yes! Sorry if that wasn't clear! The fact that MDX wasn't a true superset was a pain point and one reason I never moved my own content. And our Markdoc integration similarly didn't (before your contribution) intuitively allow for it. I think this is a great and helpful contribution, and sorry my joking tone sounded a bit dismissive. This is literally something that would make me consider switching myself. 🙂 |
|
Alright, feeling good about this team! Merging away 🚢 |
Awesome, thanks everyone! Hopefully I'll have a chance for one or two more Markdoc PRs soon (to explore making the renderer a reusable component) |
|
@alex-sherwin Nice! Really interested in that one 🙌 |
Changes
Testing
With extensive unit tests on parsed HTML DOM elements
Docs
Roadmap for this discussion: withastro/roadmap#613