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

Run yarn during ejecting to avoid hoisting issues and fix CI #3806

Merged
merged 4 commits into from
Jan 16, 2018
Merged

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Jan 15, 2018

Our CI started failing, possibly after Jest released a 21.1.0 update.

After chatting to @BYK our best guess is that because we didn't re-run Yarn after ejecting, the new script location (in the root /scripts folder rather than in node_modules/react-scripts/scripts) started picking up a different babel-core version that was previously being hoisted (6.x judging by the stack trace that has /lib/ in it) than the one react-scripts wanted (7.0 bridge).

Our current behavior always seemed weird to me (#3356), and now that it's possibly causing bugs, I think we should try getting rid of that special case. Perhaps those issues in the TODO are not relevant anymore, or maybe we can find better fixes for them. Conceptually re-running yarn sounds like the only right thing to do.

I'm kind of shooting in the dark here so I also tried to update other Jest packages while we're at it.

@Timer
Copy link
Contributor

Timer commented Jan 15, 2018

Could never get this working on windows: #3347

@gaearon
Copy link
Contributor Author

gaearon commented Jan 15, 2018

We somehow run Yarn from Create React App itself, don't we? So maybe we could run it in the same way?

@gaearon
Copy link
Contributor Author

gaearon commented Jan 15, 2018

If this passes Travis but doesn't work on Windows we'll debug further with @BYK tomorrow.

@gaearon
Copy link
Contributor Author

gaearon commented Jan 15, 2018

Ooh maybe this is because we delete react-scripts before the script itself finishes executing? That would explain why it doesn't happen for CRA.

@gaearon
Copy link
Contributor Author

gaearon commented Jan 15, 2018

For future reference, what likely happens here is that running yarn inside of yarn eject removes the currently running node_modules/.bin/react-scripts.cmd and Windows doesn't like that.

@gaearon
Copy link
Contributor Author

gaearon commented Jan 16, 2018

This works. Travis has some temporary issues but they're not our fault.

@gaearon gaearon changed the title Try to fix CI Run yarn during ejecting to avoid hoisting issues and fix CI Jan 16, 2018
@Timer Timer deleted the fix-ci branch January 22, 2018 14:13
akstuhl pushed a commit to akstuhl/create-react-app that referenced this pull request Mar 15, 2018
* Try to fix CI

* Bump Jest elsewhere

* Bump Babel elsewhere

* Fix CI on Windows by writing .cmd file back
zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Sep 30, 2018
* Try to fix CI

* Bump Jest elsewhere

* Bump Babel elsewhere

* Fix CI on Windows by writing .cmd file back
@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

Successfully merging this pull request may close these issues.

3 participants