-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
feat: JSX Support #3038
feat: JSX Support #3038
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
function h(factory, props, ...children) { | ||
return {factory, props, children} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not linted properly. We should ensure the linting covers these files as well. |
||
} | ||
const View = () => ( | ||
<div class="deno">land</div> | ||
) | ||
console.log(<View />) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{ factory: [Function: View], props: null, children: [] } |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,8 @@ | |
"sourceMap": true, | ||
"strict": true, | ||
"target": "esnext", | ||
"jsx": "react", | ||
"jsxFactory": "h", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am concerned we are not using the default here of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think either is OK. |
||
"types": [] | ||
}, | ||
"files": [ | ||
|
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.
Are you sure about this. This will leave the
.jsx
untouched if thecheckJs
compiler flag is not set. I can't see that being desirable.We should have tests around
.jsx
and we should have an expected behaviour of how they are handled. We also need it consistent with how the compiler behaves, where by default we have allow JS but not check JS, so how does that effect the emit?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.
Oh, I absolutely forgot it. Hmm... should we compile
.jsx
files as TSX?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.
Well, not compile as TSX, more that we will always attempt to transpile them, move it to the previous match clause, but I would check that that works as expected, when the
checkJs
compiler flag is not enabled (which is the default), which the integration test should prove.