feat(mdxish): preserve jsx expressions in ast processor#1332
feat(mdxish): preserve jsx expressions in ast processor#1332kevinports merged 4 commits intonextfrom
Conversation
| const encoded = base64Encode(templateContent); | ||
| return `${openTag}${HTML_BLOCK_CONTENT_START}${encoded}${HTML_BLOCK_CONTENT_END}${closeTag}`; | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Anyone know if these CodeQL failures have thrown before? I didn't touch any of these regexes.
There was a problem hiding this comment.
I think we saw this when developing mdxish, but we've resolved it and haven't seen it ever since. Investigating why it's throwing now all of a sudden..
There was a problem hiding this comment.
It seems that it's been happening since last week but I'm not sure why we haven't gotten any CodeQL failures in our PR's.. I think we can address them in a separate PR?
https://github.com/readmeio/markdown/runs/63172447235?pr=1332
There was a problem hiding this comment.
Ok made a ticket for this here https://linear.app/readme-io/issue/RM-15228/fix-codeql-security-warnings
| .use(mdxishHtmlBlocks) | ||
| .use(newEditorTypes ? mdxishJsxToMdast : undefined) // Convert JSX elements to MDAST types | ||
| .use(safeMode ? undefined : evaluateExpressions, { context: jsxContext }) // Evaluate MDX expressions using jsxContext | ||
| .use(variablesTextTransformer) // Parse {user.*} patterns from text |
There was a problem hiding this comment.
Do we also want to move the variable transformer as well?
There was a problem hiding this comment.
This was a mistake. Looking again at the variablesTextTransformer and seeing that it creates the readme-variable node type that we want in to the MDAST from mdxishAstProcessor.
That being said I moved it because the variablesTextTransformer actually relies on the behavior of evaluateExpressions:
evaluateExpressionstries to evaluate user.name- It fails (since user isn't in the jsxContext)
- It falls back to converting it to a text node containing {user.name}
variablesTextTransformerthen visits these text nodes, finds the {user.name} pattern, and converts it to a Variable node.
I think we'll need to update variablesTextTransformer to visit mdxTextExpression nodes instead of text nodes! Will work on this.
There was a problem hiding this comment.
@maximilianfalco this seems like it will have overlap with the serializers for the editor Variable extension you're working on.
There was a problem hiding this comment.
Ok finally got around to this 😅
I think we'll need to update variablesTextTransformer to visit mdxTextExpression nodes instead of text nodes! Will work on this.
Updated here. The fix is to have variablesTextTransformer visit both text and mdxTextExpression nodes for parsing: 28dd4bc
|
Code changes look good! This should be safe to do and make no difference to the rendering |
maximilianfalco
left a comment
There was a problem hiding this comment.
apart from the weird flaky QL warnings I think this is solid! this might actually improve downstream code in the Variable node in the new editor!
|
For the failing tests in |
@eaglethrost Yeah definitely. Now that we're parsing variable syntax into Variable nodes and resolving them at render with the |
## Version 13.4.0 ### ✨ New & Improved * **mdxish:** preserve jsx expressions in ast processor ([#1332](#1332)) ([7704aaa](7704aaa)) ### 🛠 Fixes & Updates * focus on code copy button ([#1352](#1352)) ([6853be1](6853be1)) * **stripComments:** not escape checkbox ([#1360](#1360)) ([a4226c2](a4226c2)) * preserve legacy syntax for legacy variables ([#1365](#1365)) ([d735a99](d735a99)) <!--SKIP CI-->
|
This PR was released!🚀 Changes included in v13.4.0 |
This is engine side work to support https://github.com/readmeio/readme/pull/17142/changes
Refactors the markdown processing pipeline to move
removeJSXCommentsandevaluateExpressionsfrommdxishAstProcessorto themdxishfunction.We want to preserve jsx comments/expression in the MDAST produced by
mdxishAstProcessorso the MdxishEditor can consume them!removeJSXCommentsandevaluateExpressionsshould only occur in our compiling/rendering phase.🧰 Changes
🧬 QA & Testing