-
Notifications
You must be signed in to change notification settings - Fork 20
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
Change NPM with Yarn #764
Change NPM with Yarn #764
Conversation
From now on, everything needed to run "npm run release" and "npm run i18n are "dependencies", even if they don't end up in the bundle (i.e: babel). Everything else (redbox, testing-related deps, eslint, etc) are devDependencies.
71125f8
to
be64c05
Compare
Marked as "in progress" until this issue is fixed: sass/node-sass#1804 (of course, to make things more interesting, it only happens in Travis CI) |
Nevermind, I found a workaround: not cache |
Started reviewing this, but I didn't finish everything I wanted to do (getting The changes from
This is likely an issue with using Using the usually-recommended workaround
It took 56.05s. All of the unit tests passed ✅ I have not yet tested it manually, though. I agree with your devDep/dep definition. |
That was an oversight on my part, I forgot to update the
Yep, we have some nasty stuff there. We can't update to |
Thanks! It works now with
I ran into this issue when running
It looks like the issue we're running into is yarnpkg/yarn#761 - which seems to be being fixed atm. Until it's fixed, though, I say we shouldn't merge this PR. If we end up using yarn, we should decide on a firm yarn version (i.e. replace I am slightly in favour of not switching to yarn because of these concerns:
Happy to hear a second opinion, though :) |
Doesn't happen to me, so I assumed we were fine. So much for determinism... According to the issue comments, this may or may not be solved. Could you try using the latest
Once
+1 to that. I was aiming to switch in one repo, and if everything goes fine, switch in all other repos. If
I agree it's not super-high priority (this is a One of the big selling points of So this is probably not going to happen right now, but I think we at least should keep this in the back of our heads. |
Works for me now, and the issue from earlier is marked as resolved, too.
Ah, nice. Yeh, I agree with that.
Ugh, that's annoying. Thanks for pointing it out - definitely a good consideration. Maybe even good to revisit yarn the next time we have an issue.
Definitely agree there. Let's revisit at version 1 at the very latest? |
Folks at Calypso tried this a few months ago but shelved it because Yarn wasn't mature enough. Relevant PR: Automattic/wp-calypso#8660
Many big JS projects are already using it (including Webpack and React, for example), so I guess that's mature enough to give it a try.
Yarn is a JS package manager created by Facebook. It can be used as an (almost) drop-in replacement for NPM. Its advantages over NPM are:
shrinkwrap
, but it's used by default, so there's no way to install a package and forget to updatenpm-shrinkwrap.json
or something like that.woocommerce-connect-client
from scratch to be twice as fast, or more.yarn why packageName
will show why that package is installed (which dependency or sub-sub-subdependency is using it).To install Yarn: https://yarnpkg.com/en/docs/install#mac-tab
While I was at it, I re-arranged the
dependencies
anddevDependencies
, as we were being quite erratic deciding where to put each one. Since all the code is bundled, there are no real "dependencies" per-se, so I made an executive decision: If something is needed to runnpm run i18n
andnpm run release
, then it's adependency
, else it's adevDependency
. That leaves asdevDependencies
testing-related stuff, linters and things likewebpack-dev-server
. This change can be back-ported tomaster
even if we discard the rest of this PR.You can disagree with that, of course, as long as you provide another definition ofdependency
anddevDependency
:)To test:
yarn
(shortcut toyarn install
).dependencies
are enough to build a release, removenode_modules
andyarn.lock
, then runyarn install --production && npm run release
. It should work.Note: In Windows,
yarn
has a race condition that affects thekarma
CLI. More details. This shouldn't be a gating issue IMHO.