Skip to content
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

Support index.jsx entry file #3052

Closed
nickserv opened this issue Sep 2, 2017 · 13 comments
Closed

Support index.jsx entry file #3052

nickserv opened this issue Sep 2, 2017 · 13 comments

Comments

@nickserv
Copy link
Contributor

nickserv commented Sep 2, 2017

While I understand that the jsx extension is supported but not recommended, I was surprised that my index.jsx file isn't being recognized after renaming it to pass eslint-config-airbnb's jsx extension requirement.

Is this a bug report?

Sort of, I was surprised this wasn't supported but it may be an intentionally missing feature.

Which terms did you search for in User Guide?

index.js, it's clear that it's currently expected for the filename to be an exact match.

Steps to Reproduce

  1. Rename index.js to index.jsx
  2. npm start

Expected Behavior

App runs normally, treating index.jsx as the same as index.js when it's not present.

Actual Behavior

> react-scripts start

Could not find a required file.
  Name: index.js
  Searched in: /Users/nick/Repos/password-entropy/src
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @ start: `react-scripts start`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the @ start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
@Timer
Copy link
Contributor

Timer commented Sep 4, 2017

Please see #290; we do not recommend using the jsx extension.
The use of .jsx is only supported for legacy reasons, so I'm not sure we're going to be adding support for index.jsx any time soon -- this will just complicate things imo.

If you feel strongly about this, we can petition it but it would considered a breaking change and have to be released in a major version change.

@nickserv
Copy link
Contributor Author

nickserv commented Sep 6, 2017

Thanks, I know it's not recommended, though it seems well supported enough that I would expect the jsx extension to work for the index file (especially since most index files do have JSX). It just felt a little inconsistent and confusing, though I understand it's because there is a single entry point filename.

I would be fine with dropping this if it's too complicated. However, couldn't we keep this back compatible by only searching for an index.jsx file if index.js doesn't exist (as a fallback)? I'm not suggesting we allow a root filename other than index, I'm just referring to the extension.

@variadicintegrity
Copy link

For what it's worth, Webpack uses node's module resolution for entries, which means you can use a directory or an extensionless file as an entry point. Directories resolve to an index file inside that directory. File extensions are given priority based on their order in resolve.extensions.

module.exports = {
    entry: './src', // A directory
 // entry: './src/index', // An extensionless file

    output: {
        filename: './bundle.js'
    },

    resolve: {
        extensions: ['.js', '.jsx'] // Looks for index.js first, then falls back to index.jsx
     // extensions: ['.jsx', '.js']    Looks for index.jsx first, then falls back to index.js
    }
};

So supporting this should be as simple removing the hard coded '.js' in the config.

The extensionless file might be a better bet since just using entry: './src' would instead pick up a stray src.js in the project root.

This would also allow for using .mjs files as entry points too. (If those were to be added to the extensions list) But that should probably be a separate discussion.

@Timer
Copy link
Contributor

Timer commented Sep 8, 2017

Hmm, yes -- it should be as simple as removing .js from the paths module we export. I'm not sure of any other implications or where we use it -- we'd by relying on webpack-specific module resolution though.

@nickserv
Copy link
Contributor Author

nickserv commented Sep 8, 2017

@variadicintegrity Thanks that's a good point, didn't know about that.

So we would want resolve.extensions to be ['.js', '.jsx'] to prioritize index.js in case someone has both files, right? Personally I would see this as a minor, not major (breaking), change since a project with an index.jsx file but not an index.js file would have been unsupported before anyway (but it could be a breaking change if .jsx was prioritized). Would we be able to test this out with a prerelease or something like that?

@variadicintegrity
Copy link

variadicintegrity commented Sep 9, 2017

Yes, extension order would matter.

It's true that this would rely on webpack specific module resolution, but only for the entry file.
If this project ever migrated off of webpack, or webpack changed its resolution strategy, it would be fairly trivial to get the same functionality by just checking for the existence of the various index files in the src dir.

I personally don't think that's too much of a concern, but you guys may think differently.

I also agree that it's a bit surprising for .jsx files to work everywhere except the entry file. It seems that if they're going to be reluctantly supported, they may as well be reluctantly supported everywhere.

One other tiny benefit might be that this is one less thing for a hypothetical plugin system to have to worry about. A typescript (or other compile-to-js language) plugin would need to allow .ts files as entry points anyway. Which could simply be done by adding .ts to the resolve.extensions array.

@nickserv
Copy link
Contributor Author

nickserv commented Sep 9, 2017

Totally agree.

@Timer Would you like a PR for this? If so I would prioritize .js over .jsx, which I believe makes breaking changes impossible (having an index.js and an index.jsx would always pick the former either way).

@Timer
Copy link
Contributor

Timer commented Sep 9, 2017

We prioritize .web.js over all else; while I'm sure it's rare that someone has both index.web.js and index.js, you never really know. Then we also prioritize json over jsx, which complicates things further (as an entry point, we'd obviously want jsx not json in case there was index.json).
https://github.com/facebookincubator/create-react-app/blob/5e300cebb3c656c9403f787ede5045d0a737b95f/packages/react-scripts/config/webpack.config.dev.js#L97

It seems we only use the path entry in webpack and the start/build scripts to check for its existence.
https://github.com/facebookincubator/create-react-app/blob/5e300cebb3c656c9403f787ede5045d0a737b95f/packages/react-scripts/config/paths.js#L77
https://github.com/facebookincubator/create-react-app/blob/5e300cebb3c656c9403f787ede5045d0a737b95f/packages/react-scripts/config/webpack.config.dev.js#L45-L64
https://github.com/facebookincubator/create-react-app/blob/5e300cebb3c656c9403f787ede5045d0a737b95f/packages/react-scripts/scripts/start.js#L47-L50

I'd like to see a PR implementing this where we wouldn't have to duplicate logic (like the extensions) and still retain the error message in the start.js/build.js scripts.
No guarantee it'd be merged, but can you pull something together?

@torabian
Copy link

Change your file name to index.js, takes less than 1 second!

@nickserv
Copy link
Contributor Author

nickserv commented Sep 20, 2017

@torabian This may be simple for just create-react-app alone, but it doesn't play well with developer tools. It took me at least an hour or two to get this working properly, and this affects a lot of people.

Here's what this change involves for me:

  1. Configure all Emacs modes to find JSX syntax in JS extension files. This is usually as simple as updating auto-mode-alist, but for packages like web-mode that handle multiple syntaxes, this quickly becomes complicated and difficult to debug.
  2. Configure linter to allow JSX in JS files. I use eslint-config-airbnb, so I either have to violate its requirement or configure the rule to be disabled for this file. Either way, I should not need to violate linter or style guide rules because of technology choices.
  3. If a project has any style guide documentation, you would have to document that index.jsx isn't allowed and index.js has to be used instead, causing potential confusion and additional style guide violations.

@Timer
Copy link
Contributor

Timer commented Sep 20, 2017

A temporary solution would probably be to just defer index.js to index.jsx.

index.js:

require('./index.jsx')

@nickserv
Copy link
Contributor Author

nickserv commented Sep 20, 2017

That's a great workaround, but I'm concerned about it creating confusion for contributors. If I haven't used create-react-app before and I only had experience with React and Webpack, I wouldn't be sure if I should be opening index.js or index.jsx to change functionality, and it wouldn't be clear at first glance why both need to exist.

@gaearon
Copy link
Contributor

gaearon commented Jan 8, 2018

Meh. If it's easy to fix, happy to take a PR, but I wouldn't lose sleep over this.

@gaearon gaearon closed this as completed Jan 8, 2018
lepture added a commit to lepture/create-react-app that referenced this issue Nov 1, 2018
@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants