fix(toc): prevent misnamed mdx components from breaking TOC #1242
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Discovered through [this ticket](https://linear.app/readme-io/issue/CX-2543/tabapay-toc-does-not-respect-headers-nesting) that the TOC generated by the MDX renderer would become broken with miscalculated nesting depths. This happened when a jsx element was embedded inside the MDX body and the custom component name did not match the exported jsx elements. For example: image here I'm not entirely sure whether this is an edge-case we should be guarding against from our custom component editor. Like, adding validation to ensure that the component name matches what's being exported from it. My gut tells me it'd be really hard to enforce that. So, the fix here is to simply take this edge-case into account and add protections for the cases where this happens. When a component name doesn't match the jsx element name, it previously was making its way into the `plugin/toc` flow. But b/c it has no `tagName`, the `getDepth()` calculation would break and return a `NaN` instead of a valid integer. This would break the nesting logic in `toctoHast()` and cause all headers to render at the first level. Fixed this by: 1. filtering away these mismatched components that make it through 1. additionally updating the `getDepth()` function to be more fail-proof and handle the case where `tagName` is undefined CX-2543
dannobytes
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🧰 Changes
Discovered through this ticket
that the TOC generated by the MDX renderer would become broken with
miscalculated nesting depths and exposing
h3s or other heading depths that shouldn't be included.This happened when a jsx element was embedded inside the MDX body and
the custom component name did not match the exported jsx elements.
For example:
I'm not entirely sure whether this is an edge-case we should be guarding
against from our custom component editor. Like, adding validation to
ensure that the component name matches what's being exported from it. My
gut tells me it'd be really hard to enforce that.
So, the fix here is to simply take this edge-case into account and add
protections for the cases where this happens.
When a component name doesn't match the jsx element name, it previously
was making its way into the
plugin/tocflow. But b/c it has notagName, thegetDepth()calculation would break and return aNaNinstead of a valid integer. This would break the nesting logic in
toctoHast()and cause all headers to render at the first level.Fixed this by:
getDepth()function to be more fail-proofand handle the case where
tagNameis undefined🧬 QA & Testing
Tests should pass