-
-
Notifications
You must be signed in to change notification settings - Fork 519
fix(rehype): run postprocess transformers after code highlighting #1112
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for shiki-next ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for shiki-matsu ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1112 +/- ##
==========================================
+ Coverage 88.35% 94.91% +6.56%
==========================================
Files 74 90 +16
Lines 3322 7828 +4506
Branches 956 1641 +685
==========================================
+ Hits 2935 7430 +4495
- Misses 344 392 +48
+ Partials 43 6 -37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Emm, I am not sure if this is the efficient way to do it, it's already be noted that postprocess will only be ran in certain conditions: shiki/packages/types/src/transformers.ts Lines 96 to 99 in 5068b26
I think maybe instead, we could update the docs of rehype to mention about it, and tell users to use |
|
I have updated the PR to follow your guidance: Reverted the postprocess hook execution in rehype Added inline documentation explaining why postprocess only runs with codeToHtml Provided example code showing how users can achieve HTML-based postprocessing using a root transformer with toHtml/fromHtml Removed the postprocess test and unused dependencies |
… document HTML postprocess approach
3cf701f to
b0c9ae7
Compare
antfu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we do the docs in the documentation markdown instead? And the PR still contains unrelated changes;
|
@antfu Both concerns addressed in the latest commit. Documentation moved to docs/packages/rehype.md |
Fixes the postprocess transformer hook not being called when using @shikijs/rehype.
The issue occurred because @shikijs/rehype uses codeToHast() which returns a HAST tree, but postprocess transformers expect HTML strings. This PR bridges the gap by:
Detecting when postprocess transformers are present
Converting HAST → HTML using toHtml()
Running all postprocess transformer hooks
Parsing HTML back to HAST using fromHtml()
Linked Issues
Fixes #884