Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix optional dependencies when using devDeps #170

Closed
wants to merge 1 commit into from

Conversation

zkat
Copy link
Contributor

@zkat zkat commented Mar 5, 2019

Not sure what's going on here but tests started failing after #69 got merged so I'm making a new PR to fix it.

* install: Don't omit any deps of dev deps if --only=dev

At the moment, npm will NOT install a dependency that is both a
production and a development dependency in the same tree if
`--only=dev`.

Consider the following scenario:

- `package1` has `prod-dependency` in `dependencies`
- `package1` has `dev-dependency` in `devDependencies`
- `prod-dependency` has `sub-dependency` in `dependencies`
- `dev-dependency` has `sub-dependency` in `dependencies`

So both `prod-dependency` and `dev-dependency` directly depend on
`sub-dependency`. Since `sub-dependency` is required by at least one
production dependency, then npm won't consider it as "only a dev
dependency", and therefore running `npm install --only=dev` will result
in the following tree:

```
package1/
    node_modules/
        dev-dependency
```

Notice that `sub-dependency` has ben completely omitted from the tree,
even though `dev-dependency` clearly requires it, and therefore it will
not work.

This commit makes `--only=dev` always install required dependencies of
dev dependencies, so you never end up with a broken tree.

For a more real-world reproducible example, try to run `npm install
--only=dev` in
https://github.com/resin-io/etcher/tree/a338c6e60a10aad00008febfe92e2556dcf586c6.
The resulting tree will be completely broken, but we would not have
identified this if several install scripts wouldn't fail as a result.

PR-URL: #69
Credit: @jviotti
Reviewed-By: @iarna
@zkat zkat changed the base branch from latest to release-next March 5, 2019 17:52
@isaacs isaacs force-pushed the release-next branch 2 times, most recently from 896149d to 31718e7 Compare June 29, 2019 21:55
@isaacs isaacs deleted the zkat/fix-optional branch October 2, 2020 21:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants