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

fix optional dependencies when using devDeps #170

Closed
wants to merge 1 commit into from

Commits on Mar 5, 2019

  1. install: Don't omit any deps of dev deps if --only=dev (#69)

    * 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
    jviotti authored and zkat committed Mar 5, 2019
    Configuration menu
    Copy the full SHA
    aa82717 View commit details
    Browse the repository at this point in the history