[cli] Use yarn when available #10626

mkonicek · 2016-10-29T15:29:16Z

Motivation

react-native init takes minutes even on a fast network. Yarn makes it much quicker.

When yarn is not installed on the system:

When yarn is installed:

Also added the option react-native init AwesomeApp --npm as a fallback in case yarn is not stable enough for some people (I saw some Github issues that yarn hangs for example; for me it works great though).

Test plan

Publish to Sinopia: https://github.com/facebook/react-native/tree/master/react-native-cli
react-native init AwesomeApp

Tested the following setups

New CLI - uses yarn, new react-native - uses yarn
Old CLI (1.0.0) - doesn't use yarn, new react-native - uses yarn
New CLI - uses yarn, react-native 0.36.0 - doesn't use yarn

NOTE: Yarn 14 failed with a missing manifest error: yarnpkg/yarn#454. Upgrading to Yarn 0.16.1 fixed it. To be safe let's check yarn version is at least 16.

Output (total time 45s):

$ react-native init ytest
This will walk you through creating a new React Native project in /Users/mkonicek/code/testapps/ytest
Using yarn v0.16.1
Installing react-native...
Setting up new React Native app in /Users/mkonicek/code/testapps/ytest
(node:79575) DeprecationWarning: os.tmpDir() is deprecated. Use os.tmpdir() instead.
(node:79575) DeprecationWarning: Using Buffer without `new` will soon stop working. Use `new Buffer()`, or preferably `Buffer.from()`, `Buffer.allocUnsafe()` or `Buffer.alloc()` instead.
Installing React...
Installing Jest...
warning jest > jest-cli > istanbul-api > fileset > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning Unmet peer dependency "whatwg-fetch@^1.0.0".
To run your app on iOS:
   cd /Users/mkonicek/code/testapps/ytest
   react-native run-ios
   - or -
   Open /Users/mkonicek/code/testapps/ytest/ios/ytest.xcodeproj in Xcode
   Hit the Run button
To run your app on Android:
   Have an Android emulator running (quickest way to get started), or a device connected
   cd /Users/mkonicek/code/testapps/ytest
   react-native run-android

The node DeprecationWarnings might be because I'm using node v7.0.0.

Running the generated app on iOS (reload JS works):

facebook-github-bot · 2016-10-29T15:30:00Z

By analyzing the blame information on this pull request, we identified @kentaromiura and @bestander to be potential reviewers.

bestander

A few WIP comments :)
Thanks for taking this one.

bestander · 2016-10-29T15:35:46Z

local-cli/generator/index.js


+function isYarnInstalled(boolCallback) {
+  exec('yarn --version', function (error) {
+    boolCallback(!e);


typo here !error?

bestander · 2016-10-29T15:37:39Z

local-cli/generator/index.js

-    this.npmInstall(`react@${reactVersion}`, { '--save': true, '--save-exact': true });
+    isYarnInstalled(function(hasYarn) {
+      if (hasYarn) {
+        exec(`yarn add react@${reactVersion} --exact`, function(err, stdout, stderr) {


--exact is not needed for Yarn react@${reactVersion} will do the trick

bestander · 2016-10-29T15:38:35Z

local-cli/generator/index.js

-      this.npmInstall(`jest babel-jest jest-react-native babel-preset-react-native react-test-renderer@${reactVersion}`.split(' '), {
-        saveDev: true,
-        '--save-exact': true
+      isYarnInstalled(function(hasYarn) {


I think we should run isYarnInstalled once and with execSync in the beginning of the script, it will be easier then

Good point!

bestander · 2016-10-29T15:39:59Z

react-native-cli/index.js

+  isYarnInstalled(function (hasYarn) {
+    var installCommand;
+    if (hasYarn) {
+      installCommand = 'yarn add ' + getInstallPackage(rnPackage) + ' --exact';


same here about --exact.

getInstallPackage() can return "react-native" so better to keep --exact I think?

mkonicek · 2016-10-29T16:00:33Z

Thanks a lot for the review Konstantin!

bestander

Hey Martin, the code looks fine, just a few nits.
But considering that you are going through the trouble of making it work across systems and fallback to NPM what do you think of bundling yarn.js into CLI instead?
Then no npm code is needed and we can be sure yarn is always available

bestander · 2016-10-31T23:10:02Z

local-cli/generator/index.js

+  let yarnVersion;
+  try {
+    if (process.platform.startsWith('win')) {
+      // TODO implement on Windows


Plan to implement this? :)

Nope, I don't have a Windows machine. A contributor should implement this.

http://stackoverflow.com/questions/4507312/how-to-redirect-stderr-to-null-in-cmd-exe

Thanks to @kentaromiura

What do you think of doing it in one go maybe?

bestander · 2016-10-31T23:13:44Z

local-cli/generator/index.js

+  // yarn < 0.16 has a 'missing manifest' bug
+  try {
+    if (semver.gte(yarnVersion, '0.16.0')) {
+      return yarnVersion;


Would it be better to merge 2 try-catch blocks for readability?

Also considering all the trouble...
What if we just ship yarn 0.16 with the CLI?

@bestander I don't love bundling Yarn because react-native-cli is supposed to not change very often and we put the changes in react-native/local-cli instead. As Yarn 0.17 and 0.18 etc. are released, we'll have to tell everyone to update react-native-cli too.

Also because of differences in the lockfile format and installation behavior between Yarn versions, it could be confusing if you already have Yarn installed but "rn init" creates a lockfile or node_modules that's different than what you're used to.

I suppose we could include yarn@^0.16 with r-n-cli and use it if the user doesn't have Yarn installed, but I do think it's good to first look for the user's own copy of Yarn. To me the question is: is it better to use [user's own copy of Yarn -> fallback to included Yarn 0.16] or is it better to use [user's own copy of Yarn -> fallback to user's own copy of npm]? There are good arguments to be made for both sides, on one hand we could focus on supporting just Yarn (especially for new projects), on the other using the user's own package manager might be less surprising.

good point, @ide .
In that case I agree, let's proceed as Martin started

bestander · 2016-11-01T14:35:01Z

react-native-cli/index.js


+// Use Yarn if available, it's much faster than the npm client.
+// Return the version of yarn installed on the system, null if yarn is not available.
+function yarnVersionIfAvailable() {


This really asks for being extracted into a separate file.
It is rather complex and has hardcoded versions

bestander · 2016-11-01T14:48:28Z

local-cli/generator/index.js


+// Use Yarn if available, it's much faster than the npm client.
+// Return the version of yarn installed on the system, null if yarn is not available.
+function yarnVersionIfAvailable() {


yarnVersionIfAvailable -> getYarnVersionIfAvailable

mkonicek · 2016-11-01T15:16:19Z

Fixed nits. The function getYarnVersionIfAvailable is duplicated because the files are in two separate npm packages. We could try a prepublish script to copy the file over from one to another but duplicating this bit of code seems simpler and easier to understand..

facebook-github-bot · 2016-11-01T15:19:30Z

@bestander has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

sindresorhus · 2016-11-01T15:47:23Z

local-cli/generator/index.js

+  let yarnVersion;
+  try {
+    // execSync returns a Buffer -> convert to string
+    if (process.platform.startsWith('win')) {


This should be process.platform === 'win32'. It's always win32, even on 64-bit.

Good to know, thanks! Wanted to be safe in case some future node versions start returning 'win64' or something else, even though they shouldn't as that would be a breaking change.

It will never change. win32 is the identifier for the Windows platform, not the architecture.

Summary: **Motivation** `react-native init` takes minutes even on a fast network. Yarn makes it much quicker. When yarn is not installed on the system: <img width="440" alt="screenshot 2016-10-31 22 21 19" src="https://cloud.githubusercontent.com/assets/346214/19873897/7bad5662-9fb9-11e6-85fb-ad4879949dad.png"> When yarn is installed: <img width="441" alt="screenshot 2016-10-31 22 02 20" src="https://cloud.githubusercontent.com/assets/346214/19873898/7baf4c56-9fb9-11e6-96b3-007f93d2438a.png"> Also added the option `react-native init AwesomeApp --npm` as a fallback in case yarn is not stable enough for some people (I saw some Github issues that yarn hangs for example; for me it works great though). **Test plan** 1. Publish to Sinopia: https://github.com/facebook/react-native/tree/master/react-native-cli 2. `react-native init AwesomeApp` ***Tested the following setups*** - New CLI - uses yarn, new react-native - uses yarn - Old CLI (1.0.0) - doesn't use yarn, new react-native - uses yarn - Closes #10626 Differential Revision: D4110883 Pulled By: bestander fbshipit-source-id: 8a3427e2bc9158cf5fadd8ddf5b31e4b50ce6453

f0rr0 · 2016-11-04T00:51:31Z

--verbose forces npm. Is there a verbose version for yarn?

bestander · 2016-11-04T08:06:42Z

No verbose mode in Yarn

On Friday, 4 November 2016, Siddharth Jain [email protected] wrote:

--verbose forces npm. Is there a verbose version for yarn?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#10626 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACBdWAHaaSl8o4uLJD5YV6BFkwAtswLIks5q6oHvgaJpZM4KkI8T
.

hawaiikaos · 2016-11-10T12:41:39Z

Is this resolved?

I'm using react-native for the first time, and running "react-native init [myproject]" and getting this error:

/[...]/.node/lib/node_modules/react-native-cli/index.js:81
  let yarnVersion;
  ^^^
SyntaxError: Unexpected strict mode reserved word
    at exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:443:25)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
    at startup (node.js:129:16)
    at node.js:814:3

I tried installing yarn via npm but still get the error.

hawaiikaos · 2016-11-10T12:57:47Z

Ah the above appears to be a result of this issue:

yarnpkg/yarn#1288

So it shouldn't be an issue with react-native itself.

bestander · 2016-11-10T14:17:39Z

Yeah, Yarn does not support node < 4.0

On 10 November 2016 at 12:58, Katharine Osborne [email protected]
wrote:

Ah the above appears to be a result of this issue:

yarnpkg/yarn#1288 yarnpkg/yarn#1288

So it shouldn't be an issue with react-native itself.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#10626 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACBdWP7mSFM9OwU6dLM_pcB-0WWMRW2mks5q8xT0gaJpZM4KkI8T
.

Summary: **Motivation** `react-native init` takes minutes even on a fast network. Yarn makes it much quicker. When yarn is not installed on the system: <img width="440" alt="screenshot 2016-10-31 22 21 19" src="https://cloud.githubusercontent.com/assets/346214/19873897/7bad5662-9fb9-11e6-85fb-ad4879949dad.png"> When yarn is installed: <img width="441" alt="screenshot 2016-10-31 22 02 20" src="https://cloud.githubusercontent.com/assets/346214/19873898/7baf4c56-9fb9-11e6-96b3-007f93d2438a.png"> Also added the option `react-native init AwesomeApp --npm` as a fallback in case yarn is not stable enough for some people (I saw some Github issues that yarn hangs for example; for me it works great though). **Test plan** 1. Publish to Sinopia: https://github.com/facebook/react-native/tree/master/react-native-cli 2. `react-native init AwesomeApp` ***Tested the following setups*** - New CLI - uses yarn, new react-native - uses yarn - Old CLI (1.0.0) - doesn't use yarn, new react-native - uses yarn - Closes facebook#10626 Differential Revision: D4110883 Pulled By: bestander fbshipit-source-id: 8a3427e2bc9158cf5fadd8ddf5b31e4b50ce6453

Summary: **Motivation** `react-native init` takes minutes even on a fast network. Yarn makes it much quicker. When yarn is not installed on the system: <img width="440" alt="screenshot 2016-10-31 22 21 19" src="https://cloud.githubusercontent.com/assets/346214/19873897/7bad5662-9fb9-11e6-85fb-ad4879949dad.png"> When yarn is installed: <img width="441" alt="screenshot 2016-10-31 22 02 20" src="https://cloud.githubusercontent.com/assets/346214/19873898/7baf4c56-9fb9-11e6-96b3-007f93d2438a.png"> Also added the option `react-native init AwesomeApp --npm` as a fallback in case yarn is not stable enough for some people (I saw some Github issues that yarn hangs for example; for me it works great though). **Test plan** 1. Publish to Sinopia: https://github.com/facebook/react-native/tree/master/react-native-cli 2. `react-native init AwesomeApp` ***Tested the following setups*** - New CLI - uses yarn, new react-native - uses yarn - Old CLI (1.0.0) - doesn't use yarn, new react-native - uses yarn - Closes facebook/react-native#10626 Differential Revision: D4110883 Pulled By: bestander fbshipit-source-id: 8a3427e2bc9158cf5fadd8ddf5b31e4b50ce6453

facebook-github-bot added the GH Review: review-needed label Oct 29, 2016

bestander suggested changes Oct 29, 2016

View reviewed changes

mkonicek assigned bestander Oct 29, 2016

mkonicek added GH Review: needs-revision and removed GH Review: review-needed labels Oct 29, 2016

mkonicek force-pushed the use_yarn branch from 0e84b11 to 89f7961 Compare October 29, 2016 16:14

facebook-github-bot added GH Review: review-needed and removed GH Review: needs-revision labels Oct 29, 2016

bestander reviewed Oct 31, 2016

View reviewed changes

bestander reviewed Nov 1, 2016

View reviewed changes

bestander suggested changes Nov 1, 2016

View reviewed changes

[cli] Use yarn when available

b77d72c

mkonicek force-pushed the use_yarn branch from c2ff43a to b77d72c Compare November 1, 2016 15:26

sindresorhus reviewed Nov 1, 2016

View reviewed changes

facebook-github-bot closed this in 94711bf Nov 1, 2016

ide mentioned this pull request Nov 4, 2016

Switch to Yarn in docs and CI #10345

Closed

hramos added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Nov 11, 2016

[cli] Use yarn when available #10626

[cli] Use yarn when available #10626

Uh oh!

Conversation

mkonicek commented Oct 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Oct 29, 2016

Uh oh!

bestander left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkonicek Oct 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkonicek commented Oct 29, 2016

Uh oh!

bestander left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkonicek commented Nov 1, 2016

Uh oh!

facebook-github-bot commented Nov 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkonicek Nov 1, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

f0rr0 commented Nov 4, 2016

Uh oh!

bestander commented Nov 4, 2016

Uh oh!

hawaiikaos commented Nov 10, 2016

Uh oh!

hawaiikaos commented Nov 10, 2016

Uh oh!

bestander commented Nov 10, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

mkonicek commented Oct 29, 2016 •

edited

Loading

mkonicek Oct 29, 2016 •

edited

Loading

bestander left a comment •

edited

Loading

mkonicek Nov 1, 2016 •

edited

Loading