-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
refactor(markdown): removed rehype-slug
in favor of our own implementation
#3234
Conversation
🦋 Changeset detectedLatest commit: 73e1610 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
rehype-slug
in favor of our own implementationrehype-slug
in favor of our own implementation
|
||
function rehypeCollectHeaders() { | ||
return function (tree: any) { | ||
const rehypeCollectHeaders: RehypePlugin<[]> = () => { |
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.
style nit: we usually avoid const X = () => {}
in this code base, in favor of function X() {}
.
Unless I'm missing something, you should be able to get the same type info like this:
function rehypeCollectHeaders(): RehypePlugin<[]> {
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.
Updated that!
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.
LGTM with 1 small style comment! Merge when ready
Feel free to merge if you can confirm that this PR didn't introduce this issue: withastro/docs#404 (comment) |
@FredKSchott ok, so I've added some notes about how it currently works (with or without this PR) in the documentation:
|
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.
LGTM
…ntation (withastro#3234) * Moved types arround * Removed `rehype-slug` in favor of our own implementation * Changeset * Removed rehype-slug from examples * Remove rehype-slug from tests * Updated reference * rehypeCollectHeaders is a function again * Reverted rehype-slug removes * Re-added rehype-slug to reference
Changes
While reviewing #3044, I've noticed that there is a function (
rehypeCollectHeaders
) which is being used to generate slugs no matter what plugins the user specified. So, I've removedrehype-slug
because it was doing the exact same thing (it even uses the same slugger!).Sidenote: I've moved some types around, so
headers
are typed instead ofany[]
Testing
It's already being tested
Docs
TDB