Enable JSX files extension #5233
Conversation
|
By analyzing the blame information on this pull request, we identified @cpojer, @amasad and @martinbigio to be potential reviewers. |
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
@facebook-github-bot shipit |
|
Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1938800466345905/int_phab to review. |
|
@skevy Is there anything I have to do about the |
|
An intermittent unrelated error didn't allow the commit to land. I've just restarted the process. Should land soon :) |
|
I see a "naming collision detected", looks unrelated. Merging again. We should implement retries to get around this problem where some internal tests are temporarily broken. |
|
@mkonicek it would be interesting looking at a dashboard to see why most of the pull requests fail to land. The bot already adds a special label to the issues. Besides this week due to the stability issues we had, I bet that most of the problems are because the pr needs to be rebased. It would be great that if that happens we could comment on the github issue advising the pr owner to rebase :) |
| @@ -336,6 +336,8 @@ class ResolutionRequest { | |||
| file = potentialModulePath + '.native.js'; | |||
There was a problem hiding this comment.
Unfortunately this pull request doesn't implement with .ios.jsx. But, when/if it does, it raises more questions like what happens if you have both ios.js and ios.jsx?
Sorry to chime in super late but I actually think that this is harmful to support .jsx extension. We now live in a world where we have many transforms for es6, flow, jsx... Why would we add .jsx but not .flow or .es6. Supporting more extensions is only going to fragment tools even more.
There was a problem hiding this comment.
OK. Let's revert unless there's a stronger reason to keep this diff.
There was a problem hiding this comment.
As much as I'd love to see .jsx files here you have a good point on supporting other extensions and the fragmentation it could lead too, I didn't see it that way. Thanks @vjeux
There was a problem hiding this comment.
Another late late chime in: An argument for supporting .jsx is that up until recently (e.g. pre-flow), .js pretty consistently meant "a version of ES that either matches current or future ES specs". I'm assuming that JSX will never become part of ES proper?
Similarly, why not .flow? There is .ts after all. (aside from it being potentially too late to bring up)
File extensions help tools and infrastructure determine what to do w/ the source. Having to guess is error prone and added complication
There was a problem hiding this comment.
@vjeux The specification / EPS for the ES6 module changes: https://github.com/nodejs/node-eps/blob/master/002-es6-modules.md
There was a problem hiding this comment.
Thanks! There's so much drama around this subject that we're probably going to wait until it settles and gets shipped in node before taking any action on react native
There was a problem hiding this comment.
The same is true for Jest and I'd say Facebook in general. Let's see how this goes for node and then learn from whatever they do :)
There was a problem hiding this comment.
@vjeux your first comment makes sense if you wouldn't talk about React ecosystem. I am having a hard time trying to understand why would you use that argument.
.jsx was invented by React team from the beginning of the time where React appear on the market as long as I remember from 2014 (4 years ago).
So yeah, I wouldn't support any random extension that appear but this one do not makes sense at all, React Native is about React, so probably .jsx would be more important than .js itself, but because we started using Babel we forget where we come from.
Outside that airbnb code style requires it for a really good reason, you are explicit about which file requires JSX syntax and which one doesn't and IDE/Editors likes that extension.
SeemsGood
I can't understand your point.
There was a problem hiding this comment.
temp solution. but we need built-in one!
http://www.fallingcanbedeadly.com/posts/enabling-react-native-jsx-extension
Summary: JSX extension is much used in React, this would be a natural addition for React Native. See: facebook#2303 Closes facebook#5233 Reviewed By: svcscm Differential Revision: D2818888 Pulled By: mkonicek fb-gh-sync-id: 856d1b8ba9ff88ba08a00923174e3284f359d774
Summary: This reverts commit 8887492. The original commit didn't handle cases like .(ios|android|native).js, and vjeux is in favor of standardizing on .js instead of custom extensions like .jsx or .es6 everywhere. > Unfortunately this pull request doesn't implement with .ios.jsx. But, when/if it does, it raises more questions like what happens if you have both ios.js and ios.jsx? > > Sorry to chime in super late but I actually think that this is harmful to support .jsx extension. We now live in a world where we have many transforms for es6, flow, jsx... Why would we add .jsx but not .flow or .es6. Supporting more extensions is only going to fragment tools even more. #5233 (comment) Closes #5296 Reviewed By: svcscm Differential Revision: D2830784 Pulled By: bestander fb-gh-sync-id: 9a7a4745803acbcbc5dba176b971c629c904bacd
Summary: This reverts commit 8887492. The original commit didn't handle cases like .(ios|android|native).js, and vjeux is in favor of standardizing on .js instead of custom extensions like .jsx or .es6 everywhere. > Unfortunately this pull request doesn't implement with .ios.jsx. But, when/if it does, it raises more questions like what happens if you have both ios.js and ios.jsx? > > Sorry to chime in super late but I actually think that this is harmful to support .jsx extension. We now live in a world where we have many transforms for es6, flow, jsx... Why would we add .jsx but not .flow or .es6. Supporting more extensions is only going to fragment tools even more. facebook#5233 (comment) Closes facebook#5296 Reviewed By: svcscm Differential Revision: D2830784 Pulled By: bestander fb-gh-sync-id: 9a7a4745803acbcbc5dba176b971c629c904bacd
Summary: JSX extension is much used in React, this would be a natural addition for React Native. See: facebook/react-native#2303 Closes facebook/react-native#5233 Reviewed By: svcscm Differential Revision: D2818888 Pulled By: mkonicek fb-gh-sync-id: 856d1b8ba9ff88ba08a00923174e3284f359d774
Summary: This reverts commit 888749220dac26382412f2c38ac5f9205cc842e5. The original commit didn't handle cases like .(ios|android|native).js, and vjeux is in favor of standardizing on .js instead of custom extensions like .jsx or .es6 everywhere. > Unfortunately this pull request doesn't implement with .ios.jsx. But, when/if it does, it raises more questions like what happens if you have both ios.js and ios.jsx? > > Sorry to chime in super late but I actually think that this is harmful to support .jsx extension. We now live in a world where we have many transforms for es6, flow, jsx... Why would we add .jsx but not .flow or .es6. Supporting more extensions is only going to fragment tools even more. facebook/react-native#5233 (comment) Closes facebook/react-native#5296 Reviewed By: svcscm Differential Revision: D2830784 Pulled By: bestander fb-gh-sync-id: 9a7a4745803acbcbc5dba176b971c629c904bacd
|
Im puzzled by this. Why would you want to call a file .js when it is not a proper javascript file and break all other tooling? The question isn't wether .jsx files should be supported, the obvious and in your face bug is that supporting .js files as if they are .jsx files is breaking the standard, makes tool integration harder and is generally considered to be 'HARMFULL BEHAVIOR' just like renaming a .json file to an .xml file would be. It is self centered. We may want to use tooling (linting, beautifiers) that expects the file extensions to follow the proper conventions. As nice or imporant as this project may be, it is not important enough to warrent the arrogance of diluting a standard filetype association. |
|
Not sure why this has been closed. JSX is a separate language to ES and should not use the same file extension in order to allow external tools (e.g. code formatters, linters, code metric generators, etc) to identify the type and behave appropriately. Not supporting this just because there are edge cases that might require thought is absurd. |
|
temp solution. but we need built-in one! |
|
@savvaoff why don't you make a PR about this solution? |
|
@gabrielmaiaf because that is not a problem at all. Many of us knows how to fix this but the core contributors will no take the PR fixing the issue 😢 |
|
Just so everyone one knows it is possible to use JSX files with react native. Inside of the root of your project add a file named // ./rn-cli.config.js
module.exports = {
/// @name Make ReactNative Great Again
/// @description Allows you to enable support for JSX files, and `.mjs` files which is the new node standard
/// @source http://www.fallingcanbedeadly.com/posts/enabling-react-native-jsx-extension
/// @note One caveat, The `index.js` file in the root of your project has to be `.js`.
getSourceExts: () => [ 'jsx', 'mjs', 'js' ],
} |
|
@tjbenton's solution seems to not work in newer versions of RN (I'm on |
|
In newer version just change metro.config.js |
|
module.exports = { |
|
I'm on rn-cli.config.js
metro.config.js
nothing seems to work.. |
|
Turns out the reason why module.exports = {
// Allows you to enable support for JSX files, and `.mjs` files which
// is the new node standard
// https://github.com/facebook/react-native/pull/5233
// Note: One caveat, The `index.js` file in the root of your project has to be `.js`.
resolver: {
sourceExts: [
'js', // note this has to be defined first or you get an error
'json',
'jsx',
'mjs',
// required because the react-native cli ignores `resolverMainFields`
'ts',
'tsx',
],
},
// this prevents the `react-native` field from being used so `ts` and `tsx`
// files don't have to be transpiled
// resolverMainFields: [ 'main' ],
transformer: { ... },
}Here's a screenshot of this config working for me. "metro-react-native-babel-preset": "0.56.0",
"react-native": "0.61.2",
"react": "16.9.0",
If you don't put the
|
JSX extension is much used in React, this would be a natural addition for React Native.
See: #2303