Skip to content
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.

Sign up for GitHub

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

Jump to bottom

v2 parser can't handle the open curly in JS strings #1081

Closed
johno opened this issue May 20, 2020 · 2 comments
Closed

v2 parser can't handle the open curly in JS strings #1081

johno opened this issue May 20, 2020 · 2 comments
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🐛 type/bug This is a problem 💎 v2 Issues related to v2
Milestone
v2

Comments

@johno
Copy link
Member

johno commented May 20, 2020

Currently the parser doesn't handle curlies in strings. You can test it out on the v2 playground:

# Hello, world!

<Button onClick={e => alert('hi! {')}>
  I'm a button!
</Button>
@johno johno added 🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on 💎 v2 Issues related to v2 labels May 20, 2020
@johno johno mentioned this issue May 20, 2020
Closed
25 tasks
@wooorm
Copy link
Member

wooorm commented May 21, 2020 •
edited
Loading

I’m curious to hear how often this happens in the real world!
When in strings, it’s possible to encode them:

<Button onClick={e => alert('hi! \x7b')}>
<Button onClick={e => alert('hi! \u007b')}>

@johno johno added this to the v2 milestone Jul 22, 2020
wooorm added a commit that referenced this issue Dec 11, 2020
@wooorm
Update to remark@13 (micromark)
609d9f2 
This updates MDX to use and support remark@13, which comes with a new internal
parser (micromark), and supports CommonMark.
See <https://github.com/remarkjs/remark/releases/tag/13.0.0> for more
information.
In short, this means MDX parses markdown closer to what folks expect.
And it means all latest plugins work (again).

But it also means that parsing MDX syntax (JSX, expressions, ESM) got an update.
See: <https://github.com/micromark/micromark-extension-mdxjs> and
<https://github.com/syntax-tree/mdast-util-mdx> for the syntax.
In short, this means MDX parsing is now JavaScript-aware: import/exports are
actually parsed for being valid JavaScript.
Expressions previously counted braces, but now can include braces in strings or
comments or whatnot.
This also means we can drop Babel (in a future PR) because we already have a
JavaScript AST.

This also deprecates the packages `remark-mdxjs` (which is now the default in
`remark-mdx`), `remark-mdx-remove-exports`, and `remark-mdx-remove-imports`.

Related to GH-704.
Related to GH-1041.

Closes GH-720.
Closes GH-1028.
Closes GH-1050.
Closes GH-1081.
Closes GH-1193.
Closes GH-1238.
Closes GH-1283.
Closes GH-1316.
Closes GH-1318.
Closes GH-1341.
@wooorm wooorm mentioned this issue Dec 11, 2020
Merged
wooorm added a commit that referenced this issue Dec 15, 2020
@wooorm
Update to remark@13 (micromark)
3080444 
This updates MDX to use and support remark@13, which comes with a new internal
parser (micromark), and supports CommonMark.
See <https://github.com/remarkjs/remark/releases/tag/13.0.0> for more
information.
In short, this means MDX parses markdown closer to what folks expect.
And it means all latest plugins work (again).

But it also means that parsing MDX syntax (JSX, expressions, ESM) got an update.
See: <https://github.com/micromark/micromark-extension-mdxjs> and
<https://github.com/syntax-tree/mdast-util-mdx> for the syntax.
In short, this means MDX parsing is now JavaScript-aware: import/exports are
actually parsed for being valid JavaScript.
Expressions previously counted braces, but now can include braces in strings or
comments or whatnot.
This also means we can drop Babel (in a future PR) because we already have a
JavaScript AST.

This also deprecates the packages `remark-mdxjs` (which is now the default in
`remark-mdx`), `remark-mdx-remove-exports`, and `remark-mdx-remove-imports`.

Related to GH-704.
Related to GH-1041.

Closes GH-720.
Closes GH-1028.
Closes GH-1050.
Closes GH-1081.
Closes GH-1193.
Closes GH-1238.
Closes GH-1283.
Closes GH-1316.
Closes GH-1318.
Closes GH-1341.
Closes GH-1367.

Reviewed-by: Christian Murphy <[email protected]>
@jaredpalmer jaredpalmer mentioned this issue Dec 15, 2020
Closed
@wooorm
Copy link
Member

wooorm commented Dec 18, 2020

Hi all! I’m going to close this as it landed on the main, the (now default) branch we’re using to gear up to the MDX@2 release.

The reason is so that it’s more clear which issues still need to be solved already and don’t require further work.

Expect another next release soonish, which might include it, or maybe it’s already released as a beta!

For more info, see #1041.

@wooorm wooorm closed this as completed Dec 18, 2020
@wooorm wooorm added ⛵️ status/released 🗄 area/interface This affects the public interface and removed 🙆 yes/confirmed This is confirmed and ready to be worked on labels Dec 18, 2020
@wooorm wooorm added the 💪 phase/solved Post is done label Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🐛 type/bug This is a problem 💎 v2 Issues related to v2
Milestone
v2
Development

No branches or pull requests
2 participants
@wooorm @johno