.babelrc and eject script #1806

dbismut · 2017-03-13T14:50:18Z

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?

The text was updated successfully, but these errors were encountered:

gaearon · 2017-03-13T15:31:51Z

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 · 2017-03-13T15:34:29Z

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

gaearon · 2017-03-13T15:35:27Z

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 · 2017-03-13T15:40:46Z

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:

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.
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.
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 · 2017-03-13T16:47:33Z

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 · 2017-03-13T17:40:05Z

@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 · 2017-03-13T17:54:03Z

@dbismut Can you verify whether the fix in #1810 helps?

dbismut · 2017-03-13T18:36:03Z

@gaearon It does fix it!

Timer · 2017-03-13T19:03:43Z

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

dbismut · 2017-03-13T19:06:18Z

@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 · 2017-03-13T19:07:42Z

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 · 2017-03-13T19:12:08Z

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 · 2017-03-13T19:34:13Z

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

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

gaearon · 2017-03-13T19:35:46Z

Does something like this work?

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

dbismut · 2017-03-13T19:42:07Z

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

dbismut · 2017-03-13T20:05:19Z

@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 · 2017-03-13T20:13:19Z

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 · 2017-03-13T20:16:12Z

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 · 2017-03-14T08:38:54Z

@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 #1810).

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

I have 3 questions then:

Should I add the unhandledRejection to all scripts from react-scripts?
@Timer you mentioned moving the tasks/*.sh to scripts/*.sh: do you want me to do this?
Forgive my ignorance but I have to ask: since Fixes a silent crash when ejecting #1810 modifies eject.js, can I work off the master branch or should I wait for the Fixes a silent crash when ejecting #1810 to be merged first?

Thanks!

Timer · 2017-03-14T10:20:17Z

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

Thanks for being on top of this!

dbismut · 2017-03-14T14:27:28Z

@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 · 2017-03-14T15:08:06Z

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

dbismut · 2017-03-14T15:15:38Z

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

Timer · 2017-03-14T15:16:43Z

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 · 2017-03-15T15:06:06Z

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

gaearon · 2017-03-15T15:48:58Z

Thanks @dbismut!

dbismut · 2017-03-15T15:52:51Z

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

gaearon added this to the 0.10.0 milestone Mar 13, 2017

gaearon added issue: bug tag: internal labels Mar 13, 2017

gaearon added the contributions: up for grabs! label Mar 13, 2017

dbismut mentioned this issue Mar 13, 2017

Eject doesn't work infernojs/create-inferno-app#31

Closed

gaearon mentioned this issue Mar 13, 2017

Fixes a silent crash when ejecting #1810

Merged

Timer added contributions: claimed and removed contributions: up for grabs! labels Mar 13, 2017

dbismut mentioned this issue Mar 14, 2017

Makes end-to-end testing crash on unhandled rejections #1819

Merged

dbismut closed this as completed Mar 15, 2017

lock bot locked and limited conversation to collaborators Jan 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

.babelrc and eject script #1806

.babelrc and eject script #1806

dbismut commented Mar 13, 2017 •

edited

Loading

gaearon commented Mar 13, 2017

gaearon commented Mar 13, 2017

gaearon commented Mar 13, 2017

gaearon commented Mar 13, 2017

dbismut commented Mar 13, 2017

Timer commented Mar 13, 2017

gaearon commented Mar 13, 2017

dbismut commented Mar 13, 2017

Timer commented Mar 13, 2017

dbismut commented Mar 13, 2017

Timer commented Mar 13, 2017

dbismut commented Mar 13, 2017

dbismut commented Mar 13, 2017 •

edited

Loading

gaearon commented Mar 13, 2017

dbismut commented Mar 13, 2017

dbismut commented Mar 13, 2017 •

edited

Loading

Timer commented Mar 13, 2017 •

edited

Loading

dbismut commented Mar 13, 2017

dbismut commented Mar 14, 2017

Timer commented Mar 14, 2017

dbismut commented Mar 14, 2017

Timer commented Mar 14, 2017

dbismut commented Mar 14, 2017

Timer commented Mar 14, 2017 •

edited

Loading

dbismut commented Mar 15, 2017

gaearon commented Mar 15, 2017 •

edited

Loading

dbismut commented Mar 15, 2017

.babelrc and eject script #1806

.babelrc and eject script #1806

Comments

dbismut commented Mar 13, 2017 • edited Loading

gaearon commented Mar 13, 2017

gaearon commented Mar 13, 2017

gaearon commented Mar 13, 2017

gaearon commented Mar 13, 2017

dbismut commented Mar 13, 2017

Timer commented Mar 13, 2017

gaearon commented Mar 13, 2017

dbismut commented Mar 13, 2017

Timer commented Mar 13, 2017

dbismut commented Mar 13, 2017

Timer commented Mar 13, 2017

dbismut commented Mar 13, 2017

dbismut commented Mar 13, 2017 • edited Loading

gaearon commented Mar 13, 2017

dbismut commented Mar 13, 2017

dbismut commented Mar 13, 2017 • edited Loading

Timer commented Mar 13, 2017 • edited Loading

dbismut commented Mar 13, 2017

dbismut commented Mar 14, 2017

Timer commented Mar 14, 2017

dbismut commented Mar 14, 2017

Timer commented Mar 14, 2017

dbismut commented Mar 14, 2017

Timer commented Mar 14, 2017 • edited Loading

dbismut commented Mar 15, 2017

gaearon commented Mar 15, 2017 • edited Loading

dbismut commented Mar 15, 2017

dbismut commented Mar 13, 2017 •

edited

Loading

dbismut commented Mar 13, 2017 •

edited

Loading

dbismut commented Mar 13, 2017 •

edited

Loading

Timer commented Mar 13, 2017 •

edited

Loading

Timer commented Mar 14, 2017 •

edited

Loading

gaearon commented Mar 15, 2017 •

edited

Loading