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
Adds JSXFragment to plugin-transform-typescript check for the presence of jsx #7996
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8136/ |
Nice, Can you rebase on top of master? the other code changes shouldn't be necessary because of the PR I merged earlier I believe? (or maybe I'll just do it) |
Thanks! Had some trouble to rebase it, but there you go. Think it's ok now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
@@ -279,6 +279,9 @@ export default declare((api, { jsxPragma = "React" }) => { | |||
JSXElement() { | |||
sourceFileHasJsx = true; | |||
}, | |||
JSXFragment() { | |||
sourceFileHasJsx = true; | |||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually we could just use JSX() { sourceFileHasJsx = true }
which covers both
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah or "JSXElement|JSXFragment(){}", but not sure I care
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really care either but I have a preference for the jsx since it's smaller.
That's unexpected, now the tests are failing. The JSX syntax isn't parsed anymore for some reason. Edit: test passes locally on my side |
should I regress? |
Well yes but I don't understand why it fails. |
Yes, very strange.. No clue at all of whats happening? |
I couldn't reproduce locally so it's hard to debug anyway. |
and it failed again... |
I'm looking into it, in previous commit the tests passed. I also pinged you on Slack, if you want to chat over there. |
ok, thanks |
Sorry but i'm not able to reproduce it locally. |
Just ran across a version of this bug, if you specify the /** @jsx jsx **/
/** @jsxFrag React.Fragment **/
import { jsx } from '@emotion/core'
import React from 'react'
<><p/></> Output will be: /** @jsx jsx **/
/** @jsxFrag React.Fragment **/
import { jsx } from '@emotion/core'
jsx(React.Fragment, null, jsx('p)) |
This PR Adds
JSXFragment
to plugin-transform-typescript check for type imports.