Skip to content
This repository has been archived by the owner on May 9, 2024. It is now read-only.

Add support for yarn, improve filter test #9

Merged
merged 3 commits into from
Mar 14, 2017
Merged

Conversation

OliverUv
Copy link
Contributor

Yarn support for folks who want slightly faster linking 馃槃

  • Includes a simple test for linking with yarn, requires yarn binary in $PATH (npm install -g yarn)
  • Should be entirely backwards compatible, both lib and cli. Doesn't even change output unless --yarn is given, in case any user parses output in their CI system.
  • Took the liberty of upping the version in package.json to 0.4.0 in accord with semver 2.0
  • Lib can be given any executable, but cli will only pass npm or yarn.

@OliverUv
Copy link
Contributor Author

Looks like that fixed it. Give me a heads up if you want me to squash the last two commits

@OrKoN
Copy link
Owner

OrKoN commented Mar 13, 2017

@OliverUv Looks great! I have been working on adding yarn support as well but I found out that yarn link does not install dependencies in the same way npm link does. I don't remember what the problem exactly was so I will try to recover my test files and see if it works with your PR. Thanks 馃憤

@OrKoN
Copy link
Owner

OrKoN commented Mar 13, 2017

@OliverUv which version yarn/npm/node are you using?

@OrKoN
Copy link
Owner

OrKoN commented Mar 13, 2017

Interestingly, for me locally the following test constantly fails:

  1) npm-link-shared should install dependencies via linking using yarn:

      AssertionError: lodash does not exist in module-b
      + expected - actual

      -false
      +true
      
      at basic_test (test/basic.js:23:5)
      at Context.<anonymous> (test/basic.js:43:5)

node v6.10.0, yarn 0.21.3, npm 4.4.1

@OliverUv
Copy link
Contributor Author

Wow, strange! As you can see, the node versions in Travis work (4,5,6). I'm on

~> node --version
v6.10.0
~> npm --version
3.10.10
~> yarn --version
0.21.3

@OrKoN OrKoN merged commit 4542863 into OrKoN:master Mar 14, 2017
@OrKoN
Copy link
Owner

OrKoN commented Mar 14, 2017

Merged and published! Thanks for your contribution.

@OliverUv
Copy link
Contributor Author

Fantastic, thanks! Did you figure out what the issue was on your side?

@OrKoN
Copy link
Owner

OrKoN commented Mar 14, 2017

@OliverUv no, not yet. The only thing that is different is that I am using prefix in .npmrc which points to a folder in the user's home dir. Maybe it somehow affects the linking. Also when I compare the log output I see that on my machine yarn prints a warning that the module-b is already installed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants