Skip to content

Enable JSX files extension #5233

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

Closed
kdemoya wants to merge 1 commit into from
Closed

Enable JSX files extension #5233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packager/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,8 @@ class ResolutionRequest {
file = potentialModulePath + '.native.js';
Copy link
Contributor

@vjeux vjeux Jan 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@ide ide Jan 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Let's revert unless there's a stronger reason to keep this diff.

Copy link
Contributor

@ide ide Jan 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert here: #5296

Copy link
Contributor Author

@kdemoya kdemoya Jan 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@nevir nevir Mar 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@3rd-Eden 3rd-Eden May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vjeux The specification / EPS for the ES6 module changes: https://github.com/nodejs/node-eps/blob/master/002-es6-modules.md

Copy link
Contributor

@vjeux vjeux May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@cpojer cpojer May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link

@yordis yordis Jan 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link

@andrenerd andrenerd Apr 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temp solution. but we need built-in one!
http://www.fallingcanbedeadly.com/posts/enabling-react-native-jsx-extension

} else if (this._fastfs.fileExists(potentialModulePath + '.js')) {
file = potentialModulePath + '.js';
} else if (this._fastfs.fileExists(potentialModulePath + '.jsx')) {
file = potentialModulePath + '.jsx';
} else if (this._fastfs.fileExists(potentialModulePath + '.json')) {
file = potentialModulePath + '.json';
} else {
Expand Down
2 changes: 1 addition & 1 deletion packager/react-packager/src/DependencyResolver/DependencyGraph/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class DependencyGraph {
platforms: platforms || [],
preferNativePlatform: preferNativePlatform || false,
cache,
extensions: extensions || ['js', 'json'],
extensions: extensions || ['js', 'jsx', 'json'],
mocksPattern,
extractRequires,
};
Expand Down
1 change: 1 addition & 0 deletions packager/react-packager/src/Server/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ class Server {
dir: dir,
globs: [
'**/*.js',
'**/*.jsx',
'**/*.json',
].concat(assetGlobs),
};
Expand Down