Skip to content

Switch to yarn #561

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

Merged
merged 7 commits into from
Dec 7, 2016
Merged

Switch to yarn #561

merged 7 commits into from
Dec 7, 2016

Conversation

outoftime
Copy link
Contributor

@outoftime outoftime commented Nov 18, 2016

Switches to using Yarn for package management. Yarn is the cool new NPM.

Changes:

  • Swap out npm-shrinkwrap.json for yarn.lock
  • Updates Travis configuration to install and use Yarn
  • Updates the Webpack config to deal with different module layout and inadvertently updated packages
    • Specifically, switches to whitelisting stylelint rules at the module loader level. This means that we are more resilient to new rules causing the build to break, and also has the advantage of substantially reducing bundle size
  • Updates the README installation instructions

Fixes #393

@outoftime outoftime force-pushed the yarn branch 5 times, most recently from 8e57e93 to 64e36ff Compare November 19, 2016 03:50
@outoftime
Copy link
Contributor Author

@alexpelan @jwang1919 WDYT?

@jwang1919
Copy link
Contributor

@outoftime I've never used yarn before. What's your experience so far with it?

@outoftime
Copy link
Contributor Author

outoftime commented Nov 19, 2016

@jwang1919 word, it’s really new! but we switched over at Genius a couple of weeks ago and I’m really happy with it. The main selling points are:

  • The layout of the node_modules directory is deterministic based on the contents of package.json (somewhat incredibly, this is not true of npm)
  • It’s much faster (like, an order of magnitude for a cold install)
  • It caches packages (so you can switch branches and run an install without an internet connection)
  • All operations update both package.json and yarn.lock (the latter is the Yarn equivalent of npm-shrinkwrap.json)

Big blog post here, and the home page has some good TL;DR on why it’s good

@jwang1919
Copy link
Contributor

jwang1919 commented Nov 19, 2016

Sounds good. After doing a trial run, I'm getting an error regarding fsevents again after running yarn. Similar to what happened when I ran npm install.

fsevents isn't supported on non-Mac OSX machines and is set as an optional dependency. But when yarn runs, it ignores this optional dependency rule: yarnpkg/yarn#628

Found this, not sure if we can set this on the project level: yarnpkg/yarn#789

Stacktrace:

error Error running install script for optional dependency: "C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents: Command failed.\nExit code: 1\nCommand: C:\\WINDOWS\\system32\\cmd.exe\nArguments: /d /s /c node-pre-gyp install --fallback-to-build\nDirectory: C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\nOutput:\nnode-pre-gyp info it worked if it ends with ok\nnode-pre-gyp info using [email protected]\nnode-pre-gyp info using [email protected] | win32 | x64\nnode-pre-gyp info check checked for \"C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\\lib\\binding\\Release\\node-v48-win32-x64\\fse.node\" (not found)\nnode-pre-gyp http GET https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.0.15/fse-v1.0.15-node-v48-win32-x64.tar.gz\nnode-pre-gyp http 404 https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.0.15/fse-v1.0.15-node-v48-win32-x64.tar.gz\nnode-pre-gyp ERR! Tried to download: https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.0.15/fse-v1.0.15-node-v48-win32-x64.tar.gz \nnode-pre-gyp ERR! Pre-built binaries not found for [email protected] and [email protected] (node-v48 ABI) (falling back to source compile with node-gyp) \nnode-pre-gyp http Pre-built binary not available for your system, looked for https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.0.15/fse-v1.0.15-node-v48-win32-x64.tar.gz \n\r\nC:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents>if not defined npm_config_node_gyp (node \"C:\\Users\\jwang\\AppData\\Roaming\\npm\\node_modules\\yarn\\bin\\node-gyp-bin\\\\..\\..\\node_modules\\node-gyp\\bin\\node-gyp.js\" clean )  else (node  clean ) \r\ngyp info it worked if it ends with ok\ngyp info using [email protected]\ngyp info using [email protected] | win32 | x64\ngyp info ok \n\r\nC:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents>if not defined npm_config_node_gyp (node \"C:\\Users\\jwang\\AppData\\Roaming\\npm\\node_modules\\yarn\\bin\\node-gyp-bin\\\\..\\..\\node_modules\\node-gyp\\bin\\node-gyp.js\" configure --fallback-to-build --module=C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\\lib\\binding\\Release\\node-v48-win32-x64\\fse.node --module_name=fse --module_path=C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\\lib\\binding\\Release\\node-v48-win32-x64 )  else (node  configure --fallback-to-build --module=C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\\lib\\binding\\Release\\node-v48-win32-x64\\fse.node --module_name=fse --module_path=C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\\lib\\binding\\Release\\node-v48-win32-x64 ) \r\ngyp info it worked if it ends with ok\ngyp info using [email protected]\ngyp info using [email protected] | win32 | x64\ngyp ERR! configure error \ngyp ERR! stack Error: Can't find Python executable \"python\", you can set the PYTHON env variable.\ngyp ERR! stack     at failNoPython (C:\\Users\\jwang\\AppData\\Roaming\\npm\\node_modules\\yarn\\node_modules\\node-gyp\\lib\\configure.js:449:14)\ngyp ERR! stack     at C:\\Users\\jwang\\AppData\\Roaming\\npm\\node_modules\\yarn\\node_modules\\node-gyp\\lib\\configure.js:404:11\ngyp ERR! stack     at C:\\Users\\jwang\\AppData\\Roaming\\npm\\node_modules\\yarn\\node_modules\\graceful-fs\\polyfills.js:276:29\ngyp ERR! stack     at FSReqWrap.oncomplete (fs.js:123:15)\ngyp ERR! System Windows_NT 10.0.14393\ngyp ERR! command \"C:\\\\Program Files\\\\nodejs\\\\node.exe\" \"C:\\\\Users\\\\jwang\\\\AppData\\\\Roaming\\\\npm\\\\node_modules\\\\yarn\\\\node_modules\\\\node-gyp\\\\bin\\\\node-gyp.js\" \"configure\" \"--fallback-to-build\" \"--module=C:\\\\Users\\\\jwang\\\\Documents\\\\CodeProjects\\\\popcode\\\\node_modules\\\\fsevents\\\\lib\\\\binding\\\\Release\\\\node-v48-win32-x64\\\\fse.node\" \"--module_name=fse\" \"--module_path=C:\\\\Users\\\\jwang\\\\Documents\\\\CodeProjects\\\\popcode\\\\node_modules\\\\fsevents\\\\lib\\\\binding\\\\Release\\\\node-v48-win32-x64\"\ngyp ERR! cwd C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\ngyp ERR! node -v v6.8.1\ngyp ERR! node-gyp -v v3.4.0\ngyp ERR! not ok \nnode-pre-gyp ERR! build error \nnode-pre-gyp ERR! stack Error: Failed to execute 'node-gyp.cmd configure --fallback-to-build --module=C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\\lib\\binding\\Release\\node-v48-win32-x64\\fse.node --module_name=fse --module_path=C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\\lib\\binding\\Release\\node-v48-win32-x64' (1)\nnode-pre-gyp ERR! stack     at ChildProcess.<anonymous> (C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\\node_modules\\node-pre-gyp\\lib\\util\\compile.js:83:29)\nnode-pre-gyp ERR! stack     at emitTwo (events.js:106:13)\nnode-pre-gyp ERR! stack     at ChildProcess.emit (events.js:191:7)\nnode-pre-gyp ERR! stack     at maybeClose (internal/child_process.js:877:16)\nnode-pre-gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:226:5)\nnode-pre-gyp ERR! System Windows_NT 10.0.14393\nnode-pre-gyp ERR! command \"C:\\\\Program Files\\\\nodejs\\\\node.exe\" \"C:\\\\Users\\\\jwang\\\\Documents\\\\CodeProjects\\\\popcode\\\\node_modules\\\\fsevents\\\\node_modules\\\\node-pre-gyp\\\\bin\\\\node-pre-gyp\" \"install\" \"--fallback-to-build\"\nnode-pre-gyp ERR! cwd C:\\Users\\jwang\\Documents\\CodeProjects\\popcode\\node_modules\\fsevents\nnode-pre-gyp ERR! node -v v6.8.1\nnode-pre-gyp ERR! node-pre-gyp -v v0.6.29\nnode-pre-gyp ERR! not o$ ./node_modules/.bin/bower install
'.' is not recognized as an internal or external command,
operable program or batch file.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

@jwang1919
Copy link
Contributor

Arggh still couldn't get it to work. Tried removing yarn.lock, adding .yarnrc file with ignore-optional false.

Also found that the issue might not have been fixed: yarnpkg/yarn#1059

@outoftime
Copy link
Contributor Author

@jwang1919 yarnpkg/yarn#1059 suggests that maybe ignore-scripts=true might fix it?

outoftime added a commit to outoftime/popcode that referenced this pull request Nov 19, 2016
Make Webpack configuration less sensitive to the specific layout of
`node_modules`. Extracted from
popcodeorg#561, which may need to be put
on hold until yarn properly handles optional dependencies.

Most usefully, changes the logic for loading stylelint rule modules—only
load rules that are whitelisted, instead of loading all rules unless
they’re blacklisted. Rules themselves often have external library
dependencies, so this trims down the gzipped bundle size of the linter
bundle from 415K to 355K.
outoftime added a commit to outoftime/popcode that referenced this pull request Nov 19, 2016
Make Webpack configuration less sensitive to the specific layout of
`node_modules`. Extracted from
popcodeorg#561, which may need to be put
on hold until yarn properly handles optional dependencies.

Most usefully, changes the logic for loading stylelint rule modules—only
load rules that are whitelisted, instead of loading all rules unless
they’re blacklisted. Rules themselves often have external library
dependencies, so this trims down the gzipped bundle size of the linter
bundle from 415K to 355K.
outoftime added a commit to outoftime/popcode that referenced this pull request Nov 20, 2016
Make Webpack configuration less sensitive to the specific layout of
`node_modules`. Extracted from
popcodeorg#561, which may need to be put
on hold until yarn properly handles optional dependencies.

Most usefully, changes the logic for loading stylelint rule modules—only
load rules that are whitelisted, instead of loading all rules unless
they’re blacklisted. Rules themselves often have external library
dependencies, so this trims down the gzipped bundle size of the linter
bundle from 415K to 355K.
@jwang1919
Copy link
Contributor

jwang1919 commented Nov 22, 2016

@outoftime Tried adding ignore-scripts true (yarn doesn't recognize ignore-scripts=true).
What it's doing is hiding the error but not resolving the actual issue. The same error is happening when I run gulp dev, the server starts throwing errors that it can't find the fs library.

sigh Windows problems =(

Update. Tried to be very thorough with this and tested all combinations of ignore-scripts true, ignore-optional true, ignore-scripts falseandignore-optional false`. No cigar.

@outoftime
Copy link
Contributor Author

Damn @jwang1919 that really sucks! Yarn is so much better!

OK well I will put this on hold and keep an eye on yarnpkg/yarn#1059

@outoftime
Copy link
Contributor Author

@jwang1919 I don’t suppose you have any interest in developing in an Ubuntu VM : )

@@ -0,0 +1,7589 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regenerate this file. After updating to yarn v.0.18.0 this file changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disregard.

@@ -136,13 +136,14 @@
"karma-webpack": "^1.8.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review is not for this line of code but for lines 87-93. I had to change to the following this get it to work:

  "scripts": {
    "postinstall": "\"\"./node_modules/.bin/bower\"\" install",
    "pretest": "npm run lint",
    "test": "\"\"./node_modules/.bin/karma\"\" start --single-run --no-auto-watch",
    "autotest": "\"\"./node_modules/.bin/karma\"\" start --no-single-run --auto-watch",
    "lint": "\"\"./node_modules/.bin/eslint\"\" --ext .js,.jsx --plugin react src"
  },

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, your recent changes fixed this issue.

@jwang1919
Copy link
Contributor

Okay after much waiting I can confirm that upgrading yarn to v0.18.0 will indeed pass yarn install!
Note to early Windows adopters, make sure you upgrade to v0.18.0. I had to go directly to the releases tab on github yarn to get it.
npm -global update yarn will not update to the right version, neither will downloading the Windows installer from the official yarn site.

@outoftime outoftime removed the on-hold label Dec 7, 2016
This happened for some reason. I like it.
I don’t think they’re necessary and they mess up Windows
`image-webpack-loader` contains a bunch of image processing libraries as
dependencies, most of which we don’t need. gifsicle in particular is
causing problems trying to install via yarn on Windows.

Swap in `svgo-loader`, which just wraps `svgo`, the only actual image
processor we needed.
We depend on `svgo` via `svgo-loader`, not directly, so there’s no need
to have it as an explicit dependency in `package.json`.
Travis [now officially supports
yarn](https://blog.travis-ci.com/2016-11-21-travis-ci-now-supports-yarn),
so we can just rely on its detection of `yarn.lock` and use the
shorthand for caching.
@outoftime outoftime merged commit eeefd6e into popcodeorg:master Dec 7, 2016
@outoftime outoftime deleted the yarn branch December 7, 2016 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants