Fix MDX html parsing errors #6949
Conversation
|
Thanks. This issue with text doesn't look critical and can be addressed later in a separate PR if needed. It doesn't break code. That's the usual way Prettier formats JSX and that text is JSX. |
|
@Tigge Please add a changelog entry. |
For MDX wrap JSX parsing in a fragment before sending it of to babel for parsing and formatting. In MDX adjacent JSX tags are permitted in the root. An option sent to the babel parser omits printing the root fragment and instead just prints its children. Solves prettier#6943
I've added a changelog entry |
JounQin
left a comment
JounQin
left a comment
There was a problem hiding this comment.
Can't approve this due to breaking current behavior of inline texts.
thorn0
left a comment
thorn0
left a comment
There was a problem hiding this comment.
This is not okay definitely:
Prettier pr-6949
Playground link
--parser mdxInput:
<Thingamabob>
Some text
</Thingamabob> Some textOutput:
<Thingamabob>Some text</Thingamabob>{" "}
Some
text
|
@JounQin I added a commit to this PR where I tried to address your comments. Please play with it and tell me what you think now. |
| <Hello> | ||
| test <World /> test | ||
| </Hello>123 | ||
| </Hello> |
There was a problem hiding this comment.
They're different.
Check following in playground:
<button>Here is a button</button>1There was a problem hiding this comment.
Oh. So far it's the only incompatibility between MDX's block-level JSX and React. Before trying to fix this, I'd first make sure it's not a bug.
There was a problem hiding this comment.
Yep, another evidence proving mdx is not simply jsx, right?
There was a problem hiding this comment.
The spec (which is not detailed enough unfortunately) has a clear distinction between block-level and inline JSX. And so far it looked like the block-level one is supposed to be 100% React-compatible so that it was possible to copy-paste React code from JS to MDX. Could the creators of MDX deliberately break this use case by introducing this incompatibility? I'm not sure. We need to ask them.
There was a problem hiding this comment.
There was a problem hiding this comment.
@JounQin What would you say to merging this PR as is and opening an issue about that whitespace?
There was a problem hiding this comment.
Cool to me then, but we should post an issue with reference first before merging.
|
feel free to merge 👍 |
|
A few hours at work and a lot of things can happen. Thank you all for all the input, help and updates to this merge request! |
* 'next' of github.com:prettier/prettier: Optimize some usage of `Array#filter` (#6996) Update `jest` to v24 (#6954) Replace `trim{Left,Right}` with `trim{Start,End}` (#6994) Set `trailingComma` default value to `es5` (#6963) Fix `new` usage for builtin objects (#6968) Replace `indexOf` with `includes` (#6967) fix: tests for empty type parameters in TS (#6960) Fix MDX html parsing errors (#6949) fix: issue #6813 (Zero-based lists are broken) (#6852) Style: use async functions (#6935) Disable trailingComma for Angular internal parser (#6912) Update `snapshot-diff` to v0.6.1 (#6955) Update build scripts to target Node.js 10 (#6908)
lock
bot
added
the
locked-due-to-inactivity
4 participants
For MDX wrap JSX parsing in a fragment before sending it of to babel for parsing and formatting. In MDX adjacent JSX tags are permitted in the root.
An option sent to the babel parser omits printing the root fragment and instead just prints its children.
Fixes #6943
docs/directory)changelog_unreleased/*/pr-XXXX.mdfile followingchangelog_unreleased/TEMPLATE.md.✨Try the playground for this PR✨