-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
RFC: MDX comment syntax #1042
Comments
I agree |
While adding support isn’t very technically complex, I’m relatively strongly against, because it adds mental complexity. MDX already “comes at a cost of interoperability with Markdown”, namely: HTML isn’t supported, and you get JSX in return. I don’t think there should be exceptions to that rule, as those will make it harder to reason about or explain what the language is. JSX is HTML but stricter; being strict is expected. |
I wouldn't break something that currently works in v1 without a need to do so. That being said, the fuzzy line of markdown interoperability is not ideal. |
Agreed with the original post, feels like supporting html comments is pretty important, even if actually using them is discouraged. EDIT: I have changed my mind on this, and do think that html comments should not be supported. They are not supported by jsx, which mdx compiles to. Currently, html comments "work" but are silently removed from the output, which means that they don't actually work. Since jsx as an output format doesn't support html comments, I see no reason to allow them with mdx, which is a superset of jsx. It would be trivial to release a codemod along with mdx v2 that converts html comments into jsx comments, which are valid jsx and also behave as expected -- showing up in development but being removed from the final output. |
It seems that would break remark-lint's |
Hmm, indeed. |
https://github.com/DavidAnson/markdownlint works great for markdown linting and keeping some vanilla markdown files in the same tree as mdx is very useful for my current project. The remark-based linters don't seem to cover the same rules at markdownlint does and it is a shame to lose the ability to ignore rules like line length via comment syntax ( |
@leetrout You’ll run into other problems though: MD !== MDX. markdownlint will miss stuff, and will warn about stuff that isn‘t a problem. |
Unsure why but if you remove the line break above the comment, it works. |
the MDX playground is using MDX v1 so only HTML comments will work. JSX comments only work on MDX v2. Also, HTML comments no longer work on v2. See #1039 |
I was also surprised by this and it posed a real barrier to adopting MDX for me (I have many existing markdown files with comments that I wanted to migrate to MDX and could not). So I wrote a plugin: https://github.com/leebyron/remark-comment I think I have an acceptable navigation of the whole "JSX is not HTML" issue:
I'd love to see this behavior built into MDX proper, but for now am working well, albeit with a more complex config, with this plugin. |
MDX@2 can also parse “normal” markdown. Use the extension |
Reading this thread again, the main problem folks have with HTML vs JS comments is with migrating. Either from markdown to MDX, or from v1 to v2. Not with that JS comments make more sense to HTML comments. I believe the solution for migrating from markdown to MDX is to load normal markdown too. MDX 2 can now load normal markdown. Name the file A (temporary?) solution for migrating from v1 to v2 is the plugin @leebyron made (although I think it can improved somewhat). Closing, thanks! |
As part of #1039 comment syntax currently only supports JS comment expressions:
This syntax is nice because it feels more JSX-y than HTML comments. However, it comes at a cost of interoperability with Markdown (which uses HTML comments).
I'm inclined to think ensuring that we also support HTML comments (despite it being a bit precarious to have multiple syntaxes) is worth it so that your average Markdown/README file works with MDX out of the box.
Anyone have any thoughts or opinions here?
The text was updated successfully, but these errors were encountered: