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

Added option to disable yarn on initial build to fix jest failure #1370

Closed
wants to merge 2 commits into from
Closed

Conversation

thtliife
Copy link

@thtliife thtliife commented Jan 11, 2017

When creating an app using yarn / yarnpkg, some binaries do not get properly linked, causing the Jest test scripts to fail.
If yarn is used after using npm for the initial build, everything seems to work fine.
The issue to track regarding yarn binaries should be yarnpkg/yarn#648

This PR adds a --disable-yarn option, which will force the initial build to use npm.

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@thtliife
Copy link
Author

Closing and reopening to trigger travisci rebuild

@thtliife thtliife closed this Jan 11, 2017
@thtliife thtliife reopened this Jan 11, 2017
@gaearon
Copy link
Contributor

gaearon commented Jan 11, 2017

Can you provide a detailed way to reproduce?

I don’t think we’ll be adding flags specifically for this since people who need them the most won’t know they exist anyway. If you use Yarn, you implicitly agree to be “on the edge” since it’s still new and experimental.

A detailed way to reproduce will help get it fixed sooner.

@gaearon gaearon mentioned this pull request Jan 11, 2017
12 tasks
@thtliife
Copy link
Author

@gaearon:

Can you provide a detailed way to reproduce?

I don’t think we’ll be adding flags specifically for this since people who need them the most won’t know they exist anyway. If you use Yarn, you implicitly agree to be “on the edge” since it’s still new and experimental.

A detailed way to reproduce will help get it fixed sooner.

Can you reproduce the problem with latest npm?

No, works fine with npm@4.0.5
Issue occurs if yarn is installed (Currently using yarn@0.18.1)

Description

Jest testing crashes if yarn is used to create the app using create-react-app

Steps to reproduce

Create a new app with yarn as the package manager:

  • create-react-app test-app
  • cd test-app

Run the default tests with either yarn or npm.

  • yarn run test
  • npm run test

Expected behaviour

The tests should run, and presumably pass, as no changes have yet been made, with output like:

 PASS  src/App.test.js
  ✓ renders without crashing (24ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.171s
Ran all test suites.

Watch Usage
 › Press p to filter by a filename regex pattern.
 › Press q to quit watch mode.
 › Press Enter to trigger a test run.

Actual behaviour

The test crashes, producing the following output:

Determining test suites to run...2017-01-12 09:18 node[3259] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)
2017-01-12 09:18 node[3259] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)
2017-01-12 09:18 node[3259] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)
2017-01-12 09:18 node[3259] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)
2017-01-12 09:18 node[3259] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)
2017-01-12 09:18 node[3259] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)
2017-01-12 09:18 node[3259] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)
2017-01-12 09:18 node[3259] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)
events.js:160
      throw er; // Unhandled 'error' event
      ^

Error: Error watching file for changes: EMFILE
    at exports._errnoException (util.js:1022:11)
    at FSEvent.FSWatcher._handle.onchange (fs.js:1283:11)
error Command failed with exit code 1.

Environment

$ npm ls react-scripts
test-app@0.1.0 /Users/username/projects/test-app
└── react-scripts@0.8.5 

$ node --version
v7.4.0

$ npm --version
4.0.5

$ yarn --version
0.18.1

$ uname -v
Darwin Kernel Version 16.3.0: Thu Nov 17 20:23:58 PST 2016; root:xnu-3789.31.2~1/RELEASE_X86_64

$ sw_vers
ProductName:	Mac OS X
ProductVersion:	10.12.2
BuildVersion:	16C67

@mattmarcello
Copy link

I am experiencing this issue.

@thtliife
Copy link
Author

thtliife commented Feb 8, 2017

Soo... since upgrading yarn to v0.20.0 (RC) the issue with linking of binaries appears fixed, and new apps created with cra no longer have any problems running tests...
Closing this pr, as it looks like it will not be merged anyway, and it is no longer really required.
Oh and @mattmarcello, try upgrading yarn to the 0.20.0 version, and you should see better results.

@thtliife thtliife closed this Feb 8, 2017
@gaearon
Copy link
Contributor

gaearon commented Feb 8, 2017

Thanks for the update!

@lock lock bot locked and limited conversation to collaborators Jan 21, 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.

4 participants