Skip to content
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

yarn run with bin in local node_modules #733

Closed
billxinli opened this issue Oct 11, 2016 · 27 comments
Closed

yarn run with bin in local node_modules #733

billxinli opened this issue Oct 11, 2016 · 27 comments
Labels

Comments

@billxinli
Copy link

Do you want to request a feature or report a bug?
Report a bug

What is the current behavior?
From npm with respect to npm run: In addition to the shell's pre-existing PATH, npm run adds node_modules/.bin to the PATH provided to scripts. Any binaries provided by locally-installed dependencies can be used without the node_modules/.bin prefix.

If the current behavior is a bug, please provide the steps to reproduce.

Add additional entry to the scripts section of the package.json without installing the dependency globally (in this case mocha).

  "scripts": {
    "test-api": "mocha test/api"
  },

What is the expected behavior?

If I run npm run test-api it will resolve mocha to the local copy. However yarn run test-api will produce:

$ "mocha test/api" 
sh: 1: mocha test/api: not found

Please mention your node.js, yarn and operating system version.
node: 6.7.0
yarn: 0.15
os: Ubuntu 16.04

@Daniel15
Copy link
Member

Thanks for the bug report! This is pretty important. I'll try to take a look tonight.

@notahat
Copy link

notahat commented Oct 12, 2016

Duplicate with more info here: #735

@Daniel15
Copy link
Member

@notahat - That's actually a separate issue. This issue is saying that yarn run is not running executables in the node_modules/.bin directory (eg. mocha test/api in a script should essentially run node_modules/.bin/mocha test/api if mocha is an npm dependency of the app), whereas your issue #735 is about running shell commands (I think?)

@notahat
Copy link

notahat commented Oct 12, 2016

@Daniel15 I think it's the same thing.

It's not that yarn isn't finding stuff in node_modules/.bin, it's that yarn is shelling out in such a way that the command is mocha test/api, rather than mocha with an argument of test/api. Not surprisingly, the shell can't find an executable called mocha test/api.

@Daniel15
Copy link
Member

Ohh I see! That might be it. I'll take a look tonight 😄

@slmgc
Copy link

slmgc commented Oct 12, 2016

@Daniel15 @notahat I think it's the root of the issue:

echo `npm bin` #path_to_the_project/node_modules/.bin
echo `yarn bin` #path_to_the_project/node_modules/.bin/node_modules/.bin

yarn duplicates /node_modules/.bin part.

Edit: see my next comment.

@notahat
Copy link

notahat commented Oct 12, 2016

@slmgc I don't see that when I try it.

$ cd /path/to/project
$ yarn bin
/path/to/project/node_modules/.bin

BUT, if I do this:

$ cd /path/to/project/node_modules/.bin
$ yarn bin
/path/to/project/node_modules/.bin/node_modules/.bin

That definitely isn't right, but I don't think it's the source of the problem as I'm seeing it.

@f0rr0
Copy link

f0rr0 commented Oct 12, 2016

@slmgc I don't see that either. Both paths are identical

@nodesocket
Copy link

nodesocket commented Oct 12, 2016

Both look identical for me as well.

➜  js git:(master) npm bin
/Users/justin/Sites/my-project/js/node_modules/.bin
➜  js git:(master) yarn bin
/Users/justin/Sites/my-project/js/node_modules/.bin

Though I am encountering this error (see issue 782).

@slmgc
Copy link

slmgc commented Oct 12, 2016

@notahat @f0rr0 sorry guys, you are correct: I was trying to figure out the difference and indeed pwd was path_to_the_project/node_modules/.bin. But, nevertheless, there is a difference between npm bin & yarn bin when you are inside of node_modules/.bin.

@f0rr0
Copy link

f0rr0 commented Oct 12, 2016

@notahat is correct #733 (comment). Running commands with no arguments works like pwd or ls.

@lynxluna
Copy link

As long as your command script has no arguments, it runs fine. With arguments it will fail to find the files.

"scripts": {
    "start": "node app.js",
    "test": "./node_modules/.bin/mocha",
    "bdd":  "./node_modules/.bin/cucumberjs test"
  },
  • yarn test runs fine
  • yarn start and yarn run bdd fail.

If I remove the arguments for start and bdd script, it runs fine.

@Daniel15
Copy link
Member

Hmm I'm having trouble replicating the issue on Windows.

package.json:

{
  "name": "yarntest_issue733",
  "version": "1.0.0",
  "devDependencies": {
    "jest": "^16.0.1"
  },
  "scripts": {
    "foo": "jest __tests__/hello-test.js"
  }
}

__tests__/hello-test.js is just a dummy Jest test suite:

test('foo', () => {});

Output:

C:\temp\yarntest12  ([email protected])
λ c:\src\yarn\bin\yarn run foo
yarn run v0.15.1
$ "jest hello-test.js"
 PASS  __tests__\hello-test.js
  √ foo (1ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.354s
Ran all test suites matching "hello-test.js".
Done in 2.08s.

I'll have to try it on Ubuntu again, once I figure out how to fix VirtualBox (or build a Docker image or something). It's no longer even opening on my desktop computer, and bluescreening when I start VMs on my work laptop 😕

@nodesocket
Copy link

This line looks like perhaps a culprit.

https://github.com/yarnpkg/yarn/blob/master/src/cli/commands/run.js#L64

@calvinl
Copy link

calvinl commented Oct 12, 2016

I came across this issue as well, and I did a bit of digging. It looks like this line encases cmd in double quotes, eventually sending it to child.spawn() here with the quotes intact, which causes the error.

My test script is as follows:

  "scripts": {
    "start": "echo 'hello'"
  }

In child.js:

const proc = child.spawn(program, args, opts);
console.log('program:', program, 'args:', args);

Outputs:

program: sh args: [ '-c', '"echo \'hello\'" ' ]

Where it should be:

program: sh args: [ '-c', 'echo \'hello\'' ]

Removing the double quotes in run.js seems to make it work fine.

@nodesocket
Copy link

nodesocket commented Oct 12, 2016

@AlicanC. Seems like it was intentional adding the double quotes. Found the commit using git blame.

Commit: e169535

@ticky
Copy link
Contributor

ticky commented Oct 12, 2016

It appears that npm doesn’t do the quoting e169535 introduced; seems like this feature should work the same as npm’s

@Daniel15 Daniel15 removed their assignment Oct 12, 2016
@Daniel15
Copy link
Member

Sorry I didn't get a chance to look into this more fully tonight. It looks like @ticky submitted a PR though - Thanks!

@AlicanC
Copy link
Contributor

AlicanC commented Oct 12, 2016

Yes, it's my mess. The only tested commands are currently "add", "install" and "self-update". I will add tests for "run" (and others if I have time) so this doesn't happen again.

Thanks to @billxinli for reporting and @ticky for fixing it 👍

@sebmck
Copy link
Contributor

sebmck commented Oct 13, 2016

Fixed via #809.

@sebmck sebmck closed this as completed Oct 13, 2016
@f0rr0
Copy link

f0rr0 commented Oct 13, 2016

New release soon @kittens ?

@mathieutu
Copy link

I can confirm that it's still not fixed in yarn 0.19.1.. 😞
@Daniel15 @kittens any news ?

@ticky
Copy link
Contributor

ticky commented Jan 18, 2017

@mattieutu, the fix for this was released in 0.16.0, hope it hasn't regressed :(

@mathieutu
Copy link

I don't know, but I can reproduce exactly the same problem now with the last version.

With ayarn add laravel-mix which install webpack, cross-env, and others, there is no .bin folder, and I can't launch them with package.json scripts.

I had to remove the node_modules folder and reinstall it with npm :-/

@dbpolito
Copy link

dbpolito commented Apr 4, 2017

Same here with laravel-mix

@ticky
Copy link
Contributor

ticky commented Apr 5, 2017

@mathieutu @dbpolito I suspect this is in fact an entirely separate issue where binstubs are not created for any dependency which isn’t top-level. Looks like #2758 might be your ticket?

@mathieutu
Copy link

Yeah, it seems to be exactly that thing.
I'm wondering why I thought it was this issue... I'll continue on the #2758.

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

No branches or pull requests