Warn about JSX file extension

[gaearon]

Update

We support JSX extension since 0.4.1 even though we don’t recommend it.
Read the release notes.

---

Original Thread

We don’t support it. AFAIK there is no good reason to: #87 (comment).

We should do two things:

• Document that.
• Provide a descriptive error when you attempt to import a JSX file. This should be fairly easy to do with a Webpack plugin. You can look at this project for inspiration (although obviously the solution will be different).

[tizmagik]

I'd like to work on this. I imagine the best way would be to publish a webpack plugin that takes some regex and error message as options to keep it configurable and potentially useful in other contexts. Does that approach make sense?

[gaearon]

Sounds reasonable.

[stephenkingsley]

I fix it~

[tizmagik]

Submitted #353

[gaearon]

@stephenkingsley Since @tizmagik first offered to work on this, let’s give him the opportunity to finish it?

@tizmagik I think it’s not just importing .jsx that we want to warn about. We want to warn if user imports ./App but only App.jsx exists. This is the most likely case—not that they will import ./App.jsx. So we should warn in that case. (We can check the imported extension with an ESLint rule instead, so it’s not as important.) You would want to hook into resolve mechanism like this plugin does and throw a different error.. I think.

[stephenkingsley]

We want to warn if user imports ./App but only App.jsx exists

I review @tizmagik code. when the webpack compile jsx file that we warn it. this is not you want? If had ./App.js and ./App.jsx, I think nodejs will importing ./App.js. I am a newbie, so I want to know what you want~

[gaearon]

I want the following to work:

• App.jsx exists but App.js does not
• User writes import App from './App' in another file
• We should warn

[stephenkingsley]

thx! I think @tizmagik is work too easy to fix this problem.

[stephenkingsley]

If have a solution that both have ./App.js and ./App.jsx in webpack compile? I think only have js or jsx file. so @tizmagik work I think it is ok. I want your suggestion again, please.

[gaearon]

I’m not sure I understand what you’re asking.

[stephenkingsley]

Sorry, my english is not good. User writes import App from './App'. if App.js exists and App.jsx also exists, I think webpack will import App.js file. I think this solution is right, we should not warn the User.but if App.jsx exists, App.js does not. we should warn the User. it's right?

[stephenkingsley]

checking the file that webpack compiling whether is the jsx file. I do not know what's wrong? and I think this is done what your want.

[stephenkingsley]

compiler.plugin('normal-module-factory', function (nmf) {
    nmf.plugin('after-resolve', function (data, done) {
      var pathName = data.resource.split('?')[0];
      if (pathName.match(/\/[A-Za-z0-9]+\.jsx$/)) {
        done(new Error('do not require .jsx file in ', pathName));
      } else {
        done(null, data);
      }
    });
  });

this is my idea, I need your suggestion. please help me.

[tizmagik]

@gaearon thanks for reviewing my PR. I think the way this is currently written is the cleanest/most correct way.

That is, because of these lines in webpack.config.dev.js:

resolve: {
  extensions: ['', '.js', '.json'],
  // ...
}

If a user just does import App from './App' and only App.jsx exists, Webpack will error with:

Module not found: Error: Cannot resolve 'file' or 'directory' ./App

Since .jsx was not listed as one of the resolve.extensions[]

So, instead, this plugin guards against a user specifying the .jsx extension in the import path: import App from './App.jsx' in which case they would get the configured error.

If, however, you want webpack to auto-resolve the .JSX extension, then you can add .jsx to resolve.extensions[] in the webpack config, and then this plugin will warn on import App from './App' when only App.jsx exists, as you described. However, I wouldn't recommend doing this, as it seems a roundabout way to warn about a particular file extension (e.g. allow Webpack to auto-resolve the extension, only to disallow it with a plugin).

Does that make sense?

[gaearon]

If a user just does import App from './App' and only App.jsx exists, Webpack will error with:
Module not found: Error: Cannot resolve 'file' or 'directory' ./App
Since .jsx was not listed as one of the resolve.extensions[]

I understand, and this is exactly the problem I want to solve with this issue. My proposal is a resolve plugin that checks if a .jsx file with the same name exists when Webpack can’t resolve a file, and throws a more descriptive error.

[stephenkingsley]

hahaha, I exactly knew what your means!

[tizmagik]

In that case, I wonder if it's any more or less correct to add ".jsx" to resolve.extensions[] and then have this plugin warn about using JSX files? I think it would achieve the same result. Curious to hear your thoughts...

[gaearon]

Yeah, I guess this would technically work.

[tizmagik]

Alright, in that case, I'll push up a new commit to add ".jsx" to resolve.extensions[]

[tizmagik]

I left it out of the prod webpack config, which I think makes sense.

#361 ready for review.

Thanks.

[navarrorc]

Just a question. Why all the fuzz about .jsx. Isn't it the desired extension when working with JSX? I was wondering why create-react-app uses the .js file extension for all files even if the file contains JSX. I'm just curious, thanks in advance for any explanation. 😃

Found my answer here: #87 (comment)

[gaearon]

I think that if people only use "JSX" theme in component files, they miss out on all the ES6 + beyond highlighting that "Babel" themes (often marketed as JSX) usually implement.

Ideally we should just include proper instructions on installing a Babel syntax theme.

[tizmagik]

Ideally we should just include proper instructions on installing a Babel syntax theme.

+1 for that!

Anyway, any other feedback here @gaearon or is this good to merge? #361

[gaearon]

I’ll get back to it as soon as I’m done with other things. 😉

[atellmer]

I use jsx extension because my editor( VS Code) support Emmet only for jsx files.

[tomitrescak]

If there is no support for jsx, also all typescript users are not in play as TS compiles TSX to JSX :/ It would be great to have the possibility to set the resolve. extensions bit.

[chaffeqa]

Having had a similar discussion on .jsx extensions in general, can I offer this opinion and then a solution based on it:

• Including different extensions should be a developers choice, so an opinion
• As a technology that is quickly changing, its easy to say just throw .babel.js, or .es6.js..., but when you think down the road, introducing more complexity into an already complex environment just adds more unsure-ness to those trying to keep up / pick up
• Most of the boilerplate react projects I've seen out there just use .js, makes it all less scary to someone jumping in
• Editor highlighting should not dictate the direction of a tool, it should be the other way around

Therefore, my proposed solution would be:

1. Once this tool's webpack config is extendable, allow for those extensions
2. Document how to add .jsx, .tsx, or any other extensions to babel plugins / resolvers

PS: this is coming from someone who has had to learn react from scratch in the last month and use it to inceptualize a large project's codebase

[gaearon]

Okay, let’s add support for .jsx. Will merge a PR that does that.

[tizmagik]

@gaearon #563

[gaearon]

We support JSX extension since 0.4.1 even though we don’t recommend it.
Read the release notes.