-
Notifications
You must be signed in to change notification settings - Fork 204
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
Switch package management from npm to yarn #359
Conversation
Possibly related to Travis failures: yarnpkg/yarn#1016 |
Codecov Report
@@ Coverage Diff @@
## master #359 +/- ##
=======================================
Coverage 89.42% 89.42%
=======================================
Files 121 121
Lines 4785 4785
Branches 818 818
=======================================
Hits 4279 4279
Misses 506 506 Continue to review full report at Codecov.
|
I've rebased this on master locally, tests pass. If I run diff --git a/yarn.lock b/yarn.lock
index 491e5bb5..6c984735 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -280,10 +280,6 @@ ast-types@0.8.12:
version "0.8.12"
resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.8.12.tgz#a0d90e4351bb887716c83fd637ebf818af4adfcc"
-ast-types@0.8.15:
- version "0.8.15"
- resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.8.15.tgz#8eef0827f04dff0ec8857ba925abe3fea6194e52"
-
ast-types@0.9.6:
version "0.9.6"
resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.9.6.tgz#102c9e9e9005d3e7e3829bf0c4fa24ee862ee9b9"
@@ -3849,11 +3845,7 @@ lower-case@^1.1.0, lower-case@^1.1.1, lower-case@^1.1.2:
version "1.1.4"
resolved "https://registry.yarnpkg.com/lower-case/-/lower-case-1.1.4.tgz#9a2cabd1b9e8e0ae993a4bf7d5875c39c42e8eac"
-lru-cache@2:
- version "2.7.3"
- resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-2.7.3.tgz#6d4524e8b955f95d4f5b58851ce21dd72fb4e952"
-
-lru-cache@2.2.x:
+lru-cache@2, lru-cache@2.2.x:
version "2.2.4"
resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-2.2.4.tgz#6c658619becf14031d0d0b594b16042ce4dc063d"
@@ -4895,7 +4887,7 @@ readline2@^1.0.1:
is-fullwidth-code-point "^1.0.0"
mute-stream "0.0.5"
-recast@0.10.33:
+recast@0.10.33, recast@^0.10.10:
version "0.10.33"
resolved "https://registry.yarnpkg.com/recast/-/recast-0.10.33.tgz#942808f7aa016f1fa7142c461d7e5704aaa8d697"
dependencies:
@@ -4904,15 +4896,6 @@ recast@0.10.33:
private "~0.1.5"
source-map "~0.5.0"
-recast@^0.10.10:
- version "0.10.43"
- resolved "https://registry.yarnpkg.com/recast/-/recast-0.10.43.tgz#b95d50f6d60761a5f6252e15d80678168491ce7f"
- dependencies:
- ast-types "0.8.15"
- esprima-fb "~15001.1001.0-dev-harmony-fb"
- private "~0.1.5"
- source-map "~0.5.0"
-
recast@^0.11.17:
version "0.11.23"
resolved "https://registry.yarnpkg.com/recast/-/recast-0.11.23.tgz#451fd3004ab1e4df9b4e4b66376b2a21912462d3" Should these changes be committed? |
It looks to me like what has happened in that diff is that there was a dependency on (1) This merge seems like a reasonable thing to do in response to some explicit command, but I wouldn't expect it to happen during an install if there is an existing lockfile since the whole point of a lockfile is to install exactly the described versions. 🤔 I'll investigate. |
Yeah it confused me too, on a brief read of the yarn docs it seems odd for |
Hmmm yarnpkg/yarn#570 |
|
I just read something about a new NPM version supporting most of what yarn offers... I can't find it though |
http://blog.npmjs.org/post/154473364440/npm5-specifications-and-our-rfc-process specifically mentions that npm 5 aims to improve performance and the lockfile format, which are two of the biggest pain points that yarn solves, though far from the only ones. There is a beta which can be installed side-by-side with npm stable. Good to see it happening, though in some light testing the experience (with the client repo at least) is still nowhere near as good as yarn. |
Yarn provides much faster package installation, better resilience to npm registry connection issues and most importantly for us, a much better lockfile format. This commit: 1. Replaces the npm shrinkwrap with the yarn lockfile using the following steps: 1. Checkout latest version of master 2. Remove node_modules folder and re-run `npm install` 3. Run `yarn import` to generate a lockfile 2. Modifies the Makefile, Jenkinsfile, package.json and .travis.yml scripts to run `yarn` instead of `npm`.
There were a couple of issues with the initial Yarn lockfile created via `yarn import`: 1. `yarn check` reported several inconsistencies. 2. `phantomjs-prebuilt` needed to be upgraded to resolve a compatibility issue with Yarn. This commit resolves these issues by just removing and recreating the Yarn lockfile. In the process all dependencies have been upgraded to the latest versions compatible with the constraints in package.json.
Package installation fails with Node v6.2 due to an incompatible "check-dependencies" package. See https://travis-ci.org/hypothesis/client/jobs/228937886
Update lockfile with result of running `yarn install` with yarn v0.23.2.
From that lengthy thread above, I think yarnpkg/yarn#570 (comment) is the best explanation of the intended behaviour. The TL;DR is that conceptually, I ran |
Alright, lets give it a try and see how it goes |
@robertknight Would you mind sending an email to the team, informing of this change and what they need to do to their dev envs? |
This PR switches our package management from npm to Yarn. Yarn uses the same package repository as npm but offers a saner lockfile format (in yarn.lock) which is much easier to diff and merge than the huge JSON blob that
npm-shrinkwrap.json
is. The other main benefit for CI and local development is that package installation is much faster.For local development it is still possible to run
npm
manually to install packages, but you'll only get exactly the same versions of packages that CI / prod builds use if you install with yarn.Unlike
npm
, yarn is not bundled with Node so developers will need to install that separately. I have addedyarn
to the list of prerequisites for developing the client.