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

Let remark plugins injected by Starlight plugins handle Markdown text and leaf directives #2056

Merged
merged 3 commits into from
Jun 27, 2024

Conversation

HiDeoo
Copy link
Member

@HiDeoo HiDeoo commented Jun 26, 2024

Description

#1489 added a restoration process for unhandled generic Markdown directives. To do so, such directives are transformed back so they can render verbatim.

Altho, such process is done in the same remark plugin that handles asides. This means that any other remark plugin injected by Starlight plugins through Astro integrations won't be able to handle any of theses directives as they are transformed back to their original form before being processed by such plugins.

This PR addresses this issue by moving the restoration process to a new integration injecting a new remark plugin that will handle the restoration process and this integration is added after the ones potentially injected by Starlight plugins.

Copy link

changeset-bot bot commented Jun 26, 2024

🦋 Changeset detected

Latest commit: d65a34a

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

This PR includes changesets to release 1 package
Name Type
@astrojs/starlight Patch

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

Copy link

vercel bot commented Jun 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
starlight ✅ Ready (Inspect) Visit Preview Jun 27, 2024 6:36pm

@github-actions github-actions bot added the 🌟 core Changes to Starlight’s main package label Jun 26, 2024
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks @HiDeoo! I think this looks good. Just to note, I think there can still be issues with remark plugins added by other integrations, e.g.

integrations: [starlight(), directiveIntegration()]

In this case the second integration would still add the remark plugin after Starlight restored things. If we also want to handle that case I guess instead of

integrations.push(starlightDirectivesRestorationIntegration());

https://github.com/HiDeoo/starlight/blob/493f973a9e587735402c8f8492e7c30e61076864/packages/starlight/index.ts#L79

we could push the restore integration to the end of the config.integrations array in theory. But I don’t know if that’s a good idea or not. (Not familiar with the issues this was causing.)

@HiDeoo
Copy link
Member Author

HiDeoo commented Jun 27, 2024

Totally correct, I should have been more explicit about it in the PR description but that is the reason why I tried to always specify, even in the changeset, "remark plugins injected by Starlight plugins".

I was a bit afraid of the other approach and considering I am probably the only one hitting this issue now since asides are built-in in Starlight (which is pretty much since the beginning), I thought it would be better to start with this fix and see if we ever get another report about it.

@delucis
Copy link
Member

delucis commented Jun 27, 2024

Yes, that’s fine with me! Didn’t mean to suggest we should do that, just double check the scope of the fix was intentional. No need to “fix” something until we know it’s actually broken for someone. And in some ways this is less scary by making sure our logic is all scoped to the part of the process Starlight is responsible for.

@delucis delucis added the 🌟 patch Change that triggers a patch release label Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 core Changes to Starlight’s main package 🌟 patch Change that triggers a patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants