Adds yarn support with npm fallback #270

tizmagik · 2016-10-24T20:08:05Z

Closes #254

elisechant · 2016-11-05T20:39:34Z

just why? have one or the other, won't this cause more possible dependency issues that its solves?

tizmagik · 2016-11-05T21:02:37Z

@elisechant this shouldn't cause any issues I'm aware of, do you have a specific case in mind?

elisechant · 2016-11-05T21:21:30Z

@tizmagik yarn's dependency tree is more strict than npm's without a shrink-wrap. Potentially you could have developers with environments that have dependency discrepancies. I think the solution here should look like either using npm (perhaps with shrink-wrap) or yarn.

tizmagik · 2016-11-05T22:14:13Z

Thanks for the clarification @elisechant

The main benefit of this PR is to speed up CI, especially the e2e tests. Users are free to continue using npm, even if yarn is installed globally on their machine.

The only impact on users is for the setup command where the dependencies are prescribed in the starter-kyts. If a custom starter-kyt ends up having issues with yarn then we can look into adding an option to force kyt to use npm. I'm happy to look into that. Are you aware of any starter-kyts which you know would have an issue with yarn?

With all that being said, the reason why yarn is so compelling is not only because it's faster, but also because it's precisely more strict than npm+shrinkwrap. Shrinkwrap is unfortunately notoriously inconsistent and difficult to work with, so having the option of using yarn (iff it's installed globally) I see as a real benefit.

I'm sure we'd also be happy to accept a PR that adds an option to force using npm on setup, I don't see a reason why not.

ccpricenytimes

I've been thinking a bit about this and I'm wondering if just yarn being installed is enough reason to use yarn? I'm thinking particularly about projects that will want to use private dependencies. Devs may have yarn installed but not be able to use it for a particular project. Do you think it's worth investigating a way to allow users to force npm? I'm not sure how exactly it would work since kytConfig doesnt exist during setup etc. Maybe a flag? Not sure where else you would need to specify
cc @jaredmcdonald @delambo

ccpricenytimes · 2016-11-08T17:36:00Z

cli/actions/setup.js

@@ -19,6 +20,7 @@ const {
 const kytPkg = require(path.join(__dirname, '../../package.json'));

 module.exports = (config, flags, args) => {
+  logger.info('kyt version: ', kytPkg.version);


what's this for?

ccpricenytimes · 2016-11-08T17:37:47Z

package.json

@@ -7,7 +7,7 @@
    "kyt": "cli/index.js"
  },
  "engines": {
-    "node": ">=6.1.0"
+    "node": ">=6.0.0"


whats the reasoning for this?

* master: (35 commits) creates local copies of packages so e2e tests use latest code (#345) adds style and script linter packages (#344) now using npm version to check for kyt-cli updates (#337) Adds kyt version option to setup command (#343) make lint-script command; lint now runs scripts and style (#339) Adds bootstrap scripts (#341) fixes test coverage command (#342) Run test from root directory (#336) Monorepo (#330) [ci skip] document Jest/Watchman issue and common workaround (#334) Catch SIGINT (#332) Fixes e2e tests (#326) Removes deprecated reflect eslint rule (#325) Upgrade Linter dependencies (#289) [doc]fix path of eslintrc and stylelintrc (#321) Update unit tests for changes made to resolve #303 (#318) adds ISSUE_TEMPLATE (#317) Adds rfc template (#316) document possible kyt setup repository url formats (#314) Overwrites default npm init test script on setup (#293) ... # Conflicts: # .travis.yml # e2e_tests/tests/cli.test.js # package.json # packages/kyt-cli/cli/actions/setup.js

tizmagik · 2016-12-14T17:07:38Z

This is technically blocked by yarn v0.18 being released.

The issue is yarn resolves relative paths incorrectly. Fixed in yarnpkg/yarn#1498

Possible Workarounds:

Resolve relative paths before building them up in the e2e test, but I'd rather leave them as is.
Keep using npm for the e2e tests, this will require specifying which package manager to use on setup which I'm looking into adding now anyway.

* master: adds the babel presets to the bootstrap (#350) removing temp hack (#348) support user .babelrc (#347) # Conflicts: # .travis.yml

* master: not sure why but this rule is choking on travis but not locally reinstating update and hardcoding bash shebang er I have no idea why that last thing didn't work; re-expand but format script commands fixes travis build better fixes travis build fixes top level lint command # Conflicts: # e2e_tests/utils/psKill.js

sean-clayton · 2016-12-30T16:54:54Z

v0.18.0 has been released (v0.18.1 is the latest as of this comment):
https://github.com/yarnpkg/yarn/releases/tag/v0.18.0

* master: ls-starter-kyts (#340) cleans up the package.jsons (#352) # Conflicts: # packages/kyt-utils/package.json

* master: Fixes typos in static-starter-kyt

… e2e tests had wrong code

…all doesnt hoist the symlink

delambo · 2017-01-22T18:57:18Z

scripts/yarn-install-modules.sh

@@ -0,0 +1,15 @@
+#!/bin/sh


Instead of creating an optional yarn-install-modules file, do you all think it would be better to add a conditional to the install-modules file that will use yarn if it exists. Something like:

if ! type "yarn" > /dev/null; then yarn 2>&1 else npm i 2>&1 fi

I use bootstrap.sh a lot to update, so it would be helpful if the install-modules script used yarn.

* master: Fix typo (#379) kyt-cli version bump (#376) updates the kyt-cli version (#370) fixes cli starter kyt path bug (#369) Adds prep for 0.4.0 release candidate (#368) Adding identity object property; fixing preprocessor bug (#367) Adds yarn support with npm fallback (#270) Fix typo (#366) Match imports by file ext in Jest test config (#363)

Adds yarn support with npm fallback

8bf6096

tizmagik added in progress code review and removed in progress labels Oct 24, 2016

tizmagik added 2 commits October 24, 2016 17:47

Install yarn as per yarnkpkg reco

a187770

Fix min node engine version when installing via yarn

6214c3d

tizmagik changed the title ~~Adds yarn support with npm fallback~~ [WIP] Adds yarn support with npm fallback Oct 24, 2016

tizmagik added the in progress label Oct 24, 2016

tizmagik and others added 11 commits October 24, 2016 21:28

Travis

a1eb28c

debug for Travis

73caf27

verbose shell

46c50bf

shell.config.silent doesn't actually do anything apparently

b175d38

debug

c9e2eef

remove some debug

4e727d2

debuggin

e6e2352

try a different approach for log

7380ed6

even move DebugGing

7f04e50

testing cache removal

5c5639f

reMoving tHe Extra LOgging StatEmentS

0efadd0

tizmagik changed the title ~~[WIP] Adds yarn support with npm fallback~~ Adds yarn support with npm fallback Nov 5, 2016

tizmagik removed the in progress label Nov 5, 2016

ignore yarn.lock

c51537b

ccpricenytimes reviewed Nov 8, 2016

View reviewed changes

yarn helper install script, yarn.lock files

b213079

tizmagik added 4 commits December 14, 2016 13:33

Adds -p arg to specify package manager

e359dee

Keep using npm in travis for now

540ce75

Merge branch 'master' into yarn

9a55776

* master: adds the babel presets to the bootstrap (#350) removing temp hack (#348) support user .babelrc (#347) # Conflicts: # .travis.yml

tizmagik and others added 7 commits January 6, 2017 23:04

Merge branch 'master' into yarn

a8b8ece

* master: ls-starter-kyts (#340) cleans up the package.jsons (#352) # Conflicts: # packages/kyt-utils/package.json

update package.json

ee148db

Merge branch 'master' into yarn

4759d71

* master: Fixes typos in static-starter-kyt

yarn in travis

e3b49eb

for travis

01e2366

jest needs to ignore kyt ‘test’ command

4f69e66

travis node 6

a6eb91b

tizmagik mentioned this pull request Jan 7, 2017

ypm #353

Closed

ccpricenytimes and others added 7 commits January 9, 2017 20:15

merge conflicts

44b5070

Fixing error where yarn was copying non local version of kyt-utils so…

6e73323

… e2e tests had wrong code

keep lock files

080a66c

Point lint-script command to ESLint in node_modules because yarn inst…

944dba9

…all doesnt hoist the symlink

lint subcommands should stay as npm

7fa157f

lock files

aab6b2c

de-lint

b500451

ccpricenytimes approved these changes Jan 10, 2017

View reviewed changes

delambo reviewed Jan 22, 2017

View reviewed changes

ccpricenytimes merged commit 743c405 into master Jan 27, 2017

ccpricenytimes deleted the yarn branch January 27, 2017 21:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds yarn support with npm fallback #270

Adds yarn support with npm fallback #270

tizmagik commented Oct 24, 2016

elisechant commented Nov 5, 2016

tizmagik commented Nov 5, 2016

elisechant commented Nov 5, 2016

tizmagik commented Nov 5, 2016

ccpricenytimes left a comment

ccpricenytimes Nov 8, 2016

ccpricenytimes Nov 8, 2016

tizmagik commented Dec 14, 2016

sean-clayton commented Dec 30, 2016

delambo Jan 22, 2017

Adds yarn support with npm fallback #270

Adds yarn support with npm fallback #270

Conversation

tizmagik commented Oct 24, 2016

elisechant commented Nov 5, 2016

tizmagik commented Nov 5, 2016

elisechant commented Nov 5, 2016

tizmagik commented Nov 5, 2016

ccpricenytimes left a comment

Choose a reason for hiding this comment

ccpricenytimes Nov 8, 2016

Choose a reason for hiding this comment

ccpricenytimes Nov 8, 2016

Choose a reason for hiding this comment

tizmagik commented Dec 14, 2016

sean-clayton commented Dec 30, 2016

delambo Jan 22, 2017

Choose a reason for hiding this comment