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

Optional dependency with a native module is not installed #670

Closed
nebrius opened this issue Oct 11, 2016 · 11 comments
Closed

Optional dependency with a native module is not installed #670

nebrius opened this issue Oct 11, 2016 · 11 comments
Labels

Comments

@nebrius
Copy link

nebrius commented Oct 11, 2016

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

bug

What is the current behavior?

When I install johnny-five, it's optional dependency serialport with a native module (using node-pre-gyp) is missing from node_modules, but installing serialport directly works fine.

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

Using this package.json file:

{
  "name": "yarntest",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "dependencies": {
    "johnny-five": "^0.10.0"
  },
  "author": "",
  "license": "ISC"
}
  1. yarn cache clean
  2. rm yarn.lock
  3. yarn install
  4. cd node_modules
  5. find . -name serialport returns nothing

But, using this package.json file:

{
  "name": "yarntest",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "dependencies": {
    "serialport": "^4.0.0"
  },
  "author": "",
  "license": "ISC"
}
  1. yarn cache clean
  2. rm yarn.lock
  3. yarn install
  4. cd node_modules
  5. find . -name serialport shows it has been installed

What is the expected behavior?

Serialport should be installed as a dependency (the npm client works this way).

Please mention your node.js, yarn and operating system version.

Node: 6.7.0
Yarn: 0.15.1
OS: Ubuntu 14.04 x64

@sebmck sebmck added the cat-bug label Oct 11, 2016
@ljharb
Copy link

ljharb commented Oct 11, 2016

To clarify, optionalDeps are just like deps except that a failure to install or compile won't kill the overall install.

@silverwind
Copy link

Seeing the same, it looks like optional dependencies are not attempted to be built. A manual yarn add <pkg> does compile it fine tho.

@sebmck sebmck closed this as completed Oct 12, 2016
@sebmck sebmck reopened this Oct 12, 2016
@naholyr
Copy link

naholyr commented Oct 12, 2016

confirmed here:

  • yarn add hiredis --optional (properly installs, builds, and adds to package.json under optionalDependencies)
  • node -e 'require("hiredis")' (ok)
  • rm -rf node_modules
  • yarn
  • node -e 'require("hiredis")' (fails: module not found)

@shepmaster
Copy link

optionalDeps are just like deps except that a failure to install or compile won't kill the overall install.

Is there some way of knowing if a failure to compile has occurred? For example, my output looks like:

$ yarn
yarn install v0.15.1
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
success Saved lockfile.
✨  Done in 1.28s.

No obvious errors are shown.

@shepmaster
Copy link

shepmaster commented Oct 14, 2016

This also applies to optional dependencies with native code in our own package.json:

{
  "name": "scratch",
  "version": "0.0.1",
  "description": "Reproduction",
  "main": "index.js",
  "license": "MIT",
  "optionalDependencies": {
    "node-zopfli": "^2.0.1"
  }
}
$ rm -rf node_modules
$ yarn
yarn install v0.15.1
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
success Saved lockfile.
✨  Done in 1.34s.
$ find node_modules -name '*zopfli*'
node_modules/.bin/zopfli
node_modules/.bin/zopflipng

It is interesting to note that the binaries get installed, but the library itself doesn't. Running the binaries fails due to unsatisfied dependencies.

@shepmaster
Copy link

shepmaster commented Oct 14, 2016

Checking out the package.json for node-zopfli, there's an install script:

{
  "scripts": {
    "install": "node-pre-gyp install --fallback-to-build",
  },
}

Which... has a space in it! I think that at the root, this issue is yet another thing fixed by #809. The unique problem here might be the failure to run install is silently ignored because it is an optional dependency. It might be nice to have a bigger warning message in that case... not sure.

Following the instructions in this comment and this comment to run the master branch of yarn:

git clone git://github.com/yarnpkg/yarn.git
cd yarn
yarn install
npm run build
npm link

I see that the native dependency is now installed when I run yarn in my repository.

Now we just gotta wait for that commit to be released in an official version.

@smithrobs
Copy link

smithrobs commented Oct 14, 2016

Pulling master of yarn as @shepmaster suggested resolved this for me as well (my issue was noble and xpc-connection). 👍

@ashconnell
Copy link

ashconnell commented Oct 18, 2016

Which commit was this fixed in? @shepmaster

I have the same problem with this simple package.json

"devDependencies": {
  "babel-cli": "^6.16.0",
  "babel-preset-es2015": "^6.16.0",
  "babel-preset-stage-0": "^6.16.0"
}

Chokidar doesn't get installed

@shepmaster
Copy link

Which commit was this fixed in?

Please read the previous comment:

I think that at the root, this issue is yet another thing fixed by #809.

@michel-kraemer
Copy link

This issue seems to be fixed in 0.16.0.

@nebrius
Copy link
Author

nebrius commented Oct 22, 2016

Confirmed this is fixed in 0.16.1, thanks!

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

9 participants