carbon-design-system / carbon-platform

The "next" version of the Carbon Design System website, as a platform.
https://next.carbondesignsystem.com
Apache License 2.0
21 stars 5 forks source link

"<`" in MDX source causes error #1182

Closed mattrosno closed 2 years ago

mattrosno commented 2 years ago

Description

In trying to render this remote MDX: https://raw.githubusercontent.com/carbon-design-system/carbon-website/carbon-platform/src/pages/developing/angular-tutorial/step-5.mdx

That file has "<`" in it, which renders this error:

image

In /web-app/utils/mdx.js, before await processor.process(f), we need to make sure there's a space between all < and "`", with something like:

    f.value = f.value.replace('<`', '< `')
    compiledSource = await processor.process(f)

There could be better solutions, but this quick change worked for me to fix the error that I was seeing.

mattrosno commented 2 years ago

I'm adding to v1 release, because this causes an entire page to be busted.

francinelucca commented 2 years ago

This is fine by me, my only input is we'd probably want to do this in the attacher of the mdx-sanitizer plugin like we're doing with the html-commnts.

@jdharvey-ibm any thoughts on this?

mattrosno commented 2 years ago

@francinelucca ya, I have no clue what the real solution should be... just sharing my quick test to verify what was causing the error.

jharvey10 commented 2 years ago

Since this is the second instance of this we've now encountered, I'm going to put my foot down, lol.

The scenario outlined in this issue falls under the category of invalid MDX syntax. We should not attempt to diverge from the MDX specification by mutating input source. Once you put the MDX extension on a file, you're binding yourself to the rules of that language syntax.

There is a misconception that any valid Markdown is also valid MDX, but this is not the case. <`  is not valid MDX. The same thing goes for HTML comments. They are not part of the MDX specification and should not be treated as a special case.

The correct solution in both cases is to remove the offending content from the input file or for the author to modify it in a way that conforms to the MDX specification.

Right now, for HTML comments, we are making an exception because of the widespread use of formatter disable directives. These too should be removed from the source material, especially since we now have our own branch for the carbon-website material.

@francinelucca We should open an issue to replace html comments with MDX errors/warnings indicating that the syntax is not allowed and state that in the future, such content will result in a full-page error.

mattrosno commented 2 years ago

@jdharvey-ibm I'll initiate the PR to remove the offending <` from those three carbon-website pages for now, and then you and @francinelucca can open the issue for how we handle this going forward.

francinelucca commented 2 years ago

1183

jharvey10 commented 2 years ago

Thanks @mattrosno and @francinelucca! Are we okay to close out this issue in favor of #1183?

mattrosno commented 2 years ago

I'm cool with that, if the decision is that our MDX sanitization and error handling is working as designed here.