Conversation
dannobytes
left a comment
There was a problem hiding this comment.
@HugoHSun would you mind expanding the summary to include details about what the root cause of the problem is, what the proposed solution should be doing to fix it, and then any other edge-cases you can think of that are adjacent or related to it?
the customer reported this specifically for a recipe nested inside an accordian. but have you investigated deeper and checked to see if this is happening for other combinations of nested node types?
for example, what happens when you nest other block types like tables, images, code inside other components like Tabs, Cards, etc?
the fix here shouldn't just be solving the one very specific customer issue, but thinking about the more generalized problem of rendering a nested block inside a component
| const markdown = `<Accordion title="My Accordion" icon="fa-info-circle"> | ||
| <Recipe slug="recipe-title-1" title="Another One" /> | ||
| </Accordion>`; |
There was a problem hiding this comment.
what happens when there's more than one <Recipe> inside the template? should we add another test for some other edge cases?
| const tree = mdast(markdown); | ||
| const result = mdx(tree); | ||
|
|
||
| expect(result).toContain('<Recipe slug="recipe-title-1" title="Another One" />'); |
There was a problem hiding this comment.
does it make sense to only check for the presence of this? or should we make sure to check that the structure is also preserved? i.e. verify the same structure declared in the markdown test str.
| } as Recipe; | ||
|
|
||
| parent.children[index] = mdNode; | ||
| } else if (node.name in types) { |
There was a problem hiding this comment.
The recipe node went to this general handler which only keep the attributes in hProperties. But when we serialise MDAST to MDX in readmeToMdx we expect it to be in the top-level, see readme-to-mdx.ts line 54-76.
// Converts tutorial tiles to Recipe components in the migration process
// Retaining this for backwards compatibility
visit(tree, NodeTypes.tutorialTile, (tile, index, parent: Parent) => {
const { ...attrs } = tile as Recipe;
parent.children.splice(index, 1, {
type: 'mdxJsxFlowElement',
name: 'Recipe',
attributes: toAttributes(attrs, ['slug', 'title']),
children: [],
});
});
visit(tree, NodeTypes.recipe, (tile, index, parent: Parent) => {
const { ...attrs } = tile as Recipe;
parent.children.splice(index, 1, {
type: 'mdxJsxFlowElement',
name: 'Recipe',
attributes: toAttributes(attrs, ['slug', 'title']),
children: [],
});
});
Update
|
## Version 13.1.2 ### 🛠 Fixes & Updates * **magic blocks:** ensure newline characters processed as hard breaks ([#1329](#1329)) ([bb37d62](bb37d62)) * fix callout magic blocks when rendered directly below a list item ([#1331](#1331)) ([de2b82a](de2b82a)) * fix rendering content in table magic blocks ([#1318](#1318)) ([0ea1cfc](0ea1cfc)) * preserve recipe top level attributes in mdast ([#1324](#1324)) ([98f466b](98f466b)) * **stripComments:** properly pass in the micromark extensions ([#1335](#1335)) ([7ec9d46](7ec9d46)) * **mdxish:** properly terminate html blocks ([#1336](#1336)) ([d221861](d221861)) <!--SKIP CI-->
|
This PR was released!🚀 Changes included in v13.1.2 |
🧰 Changes
This bug was caused by a mismatch between
readmeComponentsandreadme-to-mdxwhen we deserialise the MDX string into Slate nodes.renderingLibrary.mdast(doc)parses the string. During this, thereadme-componentsplugin converts<Recipe slug="..." />(anmdxJsxFlowElement) into arecipenode, storing attributes only indata.hPropertiesi.e.
root
└── mdxJsxFlowElement (name: "Accordion", attributes: [title, icon])
...........└── mdxJsxFlowElement (name: "Recipe", attributes: [slug, title])
becomes
root
└── mdxJsxFlowElement (name: "Accordion", attributes: [title, icon])
...........└── recipe (data.hProperties: { slug, title }) ← attributes moved to hProperties
mdastToSlatewalks the tree. The Accordion is anmdxJsxFlowElement→ goes toJsxFlow.deserialize()JsxFlowneeds to store the JSX as a string — so it callsmdx(node)to serialize the Accordion subtree back to a string. During this, thereadme-to-mdxplugin converts therecipechild back tomdxJsxFlowElement, but looks for slug/title at the top level → not found →<Recipe />i.e.
mdxJsxFlowElement (name: "Accordion", attributes: [title, icon])
└── mdxJsxFlowElement (name: "Recipe", attributes: []) ← lost!
🧬 QA & Testing