Create-react-app: .babelrc and eject script

[dbismut]

Hey - I'm very new (or at least I'm far from understanding everything) but working on a fork of this project using inferno, I'm running into an issue with the eject script, namely that the .babelrc file can't be found.

Looking at the code of react-scripts and specifically this line, it looks like the eject script looks for a file called .babelrc (notice the dot).

During this commit .babelrc was renamed to babelrc.

I'm not running into bugs with create-react-app, but I was wondering if everything was normal?

[gaearon]

Good catch. I don’t understand why end-to-end test passes on master. 😳

This doesn’t affect our users because they’re on 0.9.x branch. That’s where stable releases are, until 0.10 is ready. I would not recommend anyone to use code in master of this repo right now.

[gaearon]

(The fix would be to hardcode Babel and ESLint "configs" since they're one liners.)

[gaearon]

This is showing up in Travis logs:

(node:3029) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: ENOENT: no such file or directory, open '/tmp/tmp.F2nYJH2DAJ/test-app/node_modules/react-scripts/.babelrc'
(node:3029) DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

We should somehow enable this behavior sooner in end-to-end test.

And I don't understand how the project still builds after ejecting on CI without those fields..

[gaearon]

OK, this is embarrassing.

Our end-to-end test doesn’t verify that ejection actually happened. So we get an unhandled Promise rejection, ejecting stops, but since it doesn’t exit with an error, the project is still in half-working state, and so it passes the rest of the end-to-end test.

If you’d like to help, please send a PR that:

1. Makes the end-to-end tests (files in tasks/*.sh) crash on unhandled rejection. Since you probably can’t enforce that from a shell file, maybe we should just do that in every scripts/*.js file. I’m not sure how to opt into this though.

2. Verify that a crash like this causes eject to exit with non-zero code. This way we can catch ejection errors in end-to-end tests instead of ignoring them.

3. Fix the actual issue here. This would mean replacing reads from those files and further assignments with something simple like

appPackage.babel = {presets: 'react-app'};
appPackage.eslintConfig = {extends: 'react-app'};

[dbismut]

Hey, thanks for investigating so quickly! I'm not sure I'm the right man for this since I've never done tests in my life (I know this is wrong, but I'll have a look and make up for it), but I was curious on point 3 why we should not read from the file and just change .babelrc to babelrc?

Unless there is absolutely no reason in the future to add other lines to babelrc and eslintrc?

[Timer]

@dbismut you can do it! We're here to help. 😄

As for point three, we simply use these files for the ejection process -- they're not used internally by react-scripts (probably why renamed, to not make devs confused when they edit them and it doesn't change anything).
We can get rid of these because they're only used in one location.

---

Looks like throwing on an unhandled rejection will be the default behavior in Node 8.

For now, looks like we can opt-in like so:

process.on('unhandledRejection', err => {
  throw err;
});

We should probably add this to scripts/*.sh, not tasks/*.sh since it will become the new default.

A PR implementing 1 and 2 would be a great start (to see an actual failure), we can talk about 3 then.

Thanks so much!

[gaearon]

@dbismut Can you verify whether the fix in https://github.com/facebookincubator/create-react-app/pull/1810 helps?

[dbismut]

@gaearon It does fix it!

[Timer]

@dbismut did you want to send a PR adding the unhandled rejection throwing to scripts?

[dbismut]

@Timer I would have loved to and currently looking into it, but by the time I understand how all this work, I'm afraid you'd be losing patience. I just figured out how to run tests from the tasks folder (not that I've been spending my whole time at this but still)...

[Timer]

Oh no, nothing like that. I just wasn't sure if you wanted to do it. Sorry if I came off that way.
Take your time. 😄 All yours!

[dbismut]

Haha no worries... I want to try, but I'll let you know if I don't think I can make it by EOD tomorrow.

[dbismut]

@Timer it looks like the below doesn't catch the error.

process.on('unhandledRejection', err => {
  throw err;
});

[gaearon]

Does something like this work?

process.on('unhandledRejection', err => {
  console.log(err);
  process.exit(1);
});

[dbismut]

Forget about what I said. What @Timer wrote actually works 🙃

[dbismut]

@Timer @gaearon it looks like that in e2e-simple.sh, from line 249, npm run eject doesn't run the script in packages/react-scripts/eject.js. If I move the npm link 252-255 lines before line 249 it actually works.

Is this another bug or just me being dumb again?

[Timer]

Hm, it should work because we run the installer from a packed copy.
See https://github.com/facebookincubator/create-react-app/blob/master/tasks/e2e-simple.sh#L128-L130 and https://github.com/facebookincubator/create-react-app/blob/master/tasks/e2e-simple.sh#L138-L143.

Is it exhibiting behavior that this doesn't happen? If so, that's interesting!
Are you using yarn? Can you try to clean your yarn cache?

[dbismut]

I'm off my desk but will try this tomorrow morning GMT. Yes I'm indeed using yarn. All I can say for now is that the code I modified in the eject.js file wasn't executed in the test!

Anyway thanks for your help, I've learned a lot today ;)

[dbismut]

@Timer Indeed you're right, running yarn cache clean solves it!

Here is the output of tasks/e2e-simple.sh after adding the unhandledRejection event to eject.js. This is when running eject.js with the wrong reading of .babelrc / .eslintrc (which is solved by https://github.com/facebookincubator/create-react-app/pull/1810).

So I guess it solves @gaearon's point number 2?

I have 3 questions then:

1. Should I add the unhandledRejection to all scripts from react-scripts?
2. @Timer you mentioned moving the tasks/*.sh to scripts/*.sh: do you want me to do this?
3. Forgive my ignorance but I have to ask: since https://github.com/facebookincubator/create-react-app/pull/1810 modifies eject.js, can I work off the master branch or should I wait for the https://github.com/facebookincubator/create-react-app/pull/1810 to be merged first?

Thanks!

[Timer]

1. Yes, please add this to all scripts.
2. No. This was my mistake, I meant scripts/*.js.
3. You should be able to make your edits before #1810 is merged. Git is really good at combining things. 👍

Thanks for being on top of this!

[dbismut]

@Timer done! However I re-ran into the yarn cache clean issue, I was wondering if cleaning yarn's cache shouldn't be part of the testing shell scripts?

[Timer]

This is a known Yarn bug (it will be fixed), see https://github.com/yarnpkg/yarn/issues/2649 and https://github.com/yarnpkg/yarn/issues/2165. I thought we clean the cache in one of them ...

[dbismut]

You do in all actually. Not sure why I ran into this issue then...

[Timer]

Ah, you need to have USE_YARN set to yes. This makes sense as these tests are meant for build servers, not so much local testing.

[dbismut]

Should be fixed by #1810 and subsequently by #1819.

[gaearon]

Thanks @dbismut!

[dbismut]

My pleasure, thanks for the help and patience, I just followed instructions 💂