-
-
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
jsxFrag should be set with latest version of @babel-plugin-transform-react-jsx #990
Comments
Based on babel/babel#11321 (comment) it seems like up until babel/babel@748897b#diff-8c3c9c6362e23bd456da3fb129f49a57L10 mdx-js has been relying on React.Fragment being passed in from preset-react. Since that has now been removed, I think a fix would be to add
Anywhere where Let me know if that makes sense and I'd be happy to do a PR. |
Note: we are restoring the old behavior of not throwing when using the preset, but I recommend enabling the fragment option since we'll re-introduce the error in Babel 8. |
We released a fix in 7.9.4 🎉! |
Thanks all! Those changes should solve this issue, right? Is there something else we should do in MDX? |
Should be solved, but keep in mind we will remove the default in Babel 8! |
Alright, I’ll close this here, and we’ll solve for it when upgrading to babel 8 when it’s out. Thanks all! |
Should we keep this open, @wooorm, and update the React loader and runtime? We'll just be specifying the current default anyway and that means we won't have to cut a new version to support babel 8 AFAIK, so we'll be more future proof. Thanks for your hard work @nicolo-ribaudo and @existentialism, we def appreciate you keeping the same behavior and waiting for babel 8 💟🙏 |
If changing something here means both current and future Babel works, then, yeah! 👍 |
Yes, explicitly setting the options should make it work both with Babel 7 and 8! |
I am not sure if this is directly related, but parcel-plugin-mdx fails bundling mdx files that use fragments. Example use case: ## MDX Test
import { State } from "react-powerplug"
<State initial={{ count: 1 }}>
{({ state, setState }) => (
<>
<p>{state.count}</p>
<button onClick={() => setState({ count: state.count + 1 })}>
increment
</button>
</>
)}
</State> Getting error:
I could also open a seperate issue.. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
These three comments above are unrelated to the original issue. The original issue was something fixed by Babel, but as they’ve changed it in their v8, we still need support for For the new comments, this seems to be a different bug around fragments. I’d appreciate it if someone could debug some more and either post a new issue or maybe even raise a PR? |
This adds a `/* @jsxFrag mdx.Fragment */` next to the existing `/* @jsx mdx */` pragma. From MDX runtimes, this exports as `mdx.Fragment` either `React.Fragment` or `Preact.Fragment`. Vue 2 does not support fragments, but as JSX and hence MDX is already specific to React or Vue, well: folks shouldn’t use fragments in MDX files targeting Vue. As we have fragments, we can also use that to pass children through missing components: `<>{props.children}</>`. This fixes runtimes where HTML is not available, such as React Native. But, as Vue doesn’t like that, there’s a hidden flag to still use the original behavior: `<div {...props} />`. Still, there remains a difference in frameworks: Vue does not put `children` in `props`, so `{...props}` has never passed children along in Vue. Closes GH-972. Closes GH-990. Closes GH-1014.
This adds a `/* @jsxFrag mdx.Fragment */` next to the existing `/* @jsx mdx */` pragma. From MDX runtimes, this exports as `mdx.Fragment` either `React.Fragment` or `Preact.Fragment`. Vue 2 does not support fragments, but as JSX and hence MDX is already specific to React or Vue, well: folks shouldn’t use fragments in MDX files targeting Vue. As we have fragments, we can also use that to pass children through missing components: `<>{props.children}</>`. This fixes runtimes where HTML is not available, such as React Native. But, as Vue doesn’t like that, there’s a hidden flag to still use the original behavior: `<div {...props} />`. Still, there remains a difference in frameworks: Vue does not put `children` in `props`, so `{...props}` has never passed children along in Vue. Closes GH-972. Closes GH-990. Closes GH-1014.
This adds a `/* @jsxFrag mdx.Fragment */` next to the existing `/* @jsx mdx */` pragma. From MDX runtimes, this exports as `mdx.Fragment` either `React.Fragment` or `Preact.Fragment`. Vue 2 does not support fragments, but as JSX and hence MDX is already specific to React or Vue, well: folks shouldn’t use fragments in MDX files targeting Vue. As we have fragments, we can also use that to pass children through missing components: `<>{props.children}</>`. This fixes runtimes where HTML is not available, such as React Native. But, as Vue doesn’t like that, there’s a hidden flag to still use the original behavior: `<div {...props} />`. Still, there remains a difference in frameworks: Vue does not put `children` in `props`, so `{...props}` has never passed children along in Vue. Closes GH-972. Closes GH-990. Closes GH-1014.
This adds a `/* @jsxFrag mdx.Fragment */` next to the existing `/* @jsx mdx */` pragma. From MDX runtimes, this exports as `mdx.Fragment` either `React.Fragment` or `Preact.Fragment`. Vue 2 does not support fragments, but as JSX and hence MDX is already specific to React or Vue, well: folks shouldn’t use fragments in MDX files targeting Vue. As we have fragments, we can also use that to pass children through missing components: `<>{props.children}</>`. This fixes runtimes where HTML is not available, such as React Native. But, as Vue doesn’t like that, there’s a hidden flag to still use the original behavior: `<div {...props} />`. Still, there remains a difference in frameworks: Vue does not put `children` in `props`, so `{...props}` has never passed children along in Vue. Closes GH-972. Closes GH-990. Closes GH-1014. Closes GH-1394. Reviewed-by: Christian Murphy <[email protected]> Reviewed-by: John Otander <[email protected]>
Subject of the issue
I'm getting the following error when compiling mdx files that use a React Fragment,
<></>
on a Gatsby site.The issue is because MDX specifies
/* @jsx mdx */ as the pragma, but never specifies a pragmaFrag which is now a requirement with
[email protected]`See the note here:
https://babeljs.io/docs/en/next/babel-plugin-transform-react-jsx.html#customizing-with-the-classic-runtime
Your environment
Steps to reproduce
Unfortunately the code is private so I can't provide it as an example, but let me know if you want me to recreate in a public repo.
Expected behaviour
MDX should set a jsxFrag value when specifying a custom jsx pragma.
So for example, every time
/* @jsx mdx */
is set/* @jsxFrag ... */
should also be set.Alternatively, babel-plugin-transform-react-jsx should use the default React.Fragment if none is specified.
Related issues:
Let me know if there's a fragment pragma that makes sense and I'd be happy to do a PR.
Actual behaviour
babel-plugin-transform-react-jsx throws an error.
The text was updated successfully, but these errors were encountered: