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 install --production' does not install transitive dependencies if listed as devDependencies in the project #2819

Closed
scinos opened this issue Mar 2, 2017 · 20 comments · Fixed by #2921

Comments

@scinos
Copy link

scinos commented Mar 2, 2017

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

What is the current behavior?

In a project A with a dependency on B and B having a dependency on C, yarn install --production wont install C if C is listed as a devDependency of A. npm does.

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

  1. Create a project with:
{
  "name": "test",
  "version": "1.0.0",
  "dependencies": {
    "sequelize-cli": "^2.5.1"
  },
  "devDependencies": {
    "gulp": "^3.9.1"
  }
}

(note that sequelize-cli has a dependency on gulp:

{
  "dependencies": {
    "gulp": "^3.9.1",
  }
}
  1. Run yarn install --production

  2. Try to find gulp: ls node_modules/gulp returns nothing.

However, if the package is

{
  "name": "test",
  "version": "1.0.0",
  "dependencies": {
    "sequelize-cli": "^2.5.1"
  }
}

yarn install --production will install gulp

What is the expected behavior?

Transitives production dependencies are always installed, even if the project declares them as devDependencies.

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

  • node 7.3.0
  • yarn 0.21.3
  • OSX 10.12.3 (Sierra)
@SimenB
Copy link
Contributor

SimenB commented Mar 2, 2017

Is #761 back again?

@razor-x
Copy link

razor-x commented Mar 6, 2017

Just hit this with yarn 0.21.3. I have a logger package (B) that depends on bunyan (C), but bunyan is a devDependency of the main package (A).

@mapsam
Copy link

mapsam commented Mar 7, 2017

Also running 0.21.3 and hitting the same issue as @razor-x

@patrickd-
Copy link

Same issue here, this just screwed a production build, not good.

@dgaubert
Copy link

dgaubert commented Mar 8, 2017

Same here using --production: main package (A) requires redis-mpool (B) which has redis (C) as dependency and main package (A) also defines redis (C) as devDependency.

@Jolg42
Copy link

Jolg42 commented Mar 10, 2017

Same here 😖 with latest 0.21.3 installing a lot of devDependecies...

@SimenB
Copy link
Contributor

SimenB commented Mar 10, 2017

Does yarn check --verify-tree report errors?

@mapsam
Copy link

mapsam commented Mar 10, 2017

@SimenB indeed it does! With the example package.json provided by @scinos above:

yarn check v0.20.3
error "sequelize-cli#gulp" not installed
error "gulp" not installed
error Found 2 errors.

@SimenB
Copy link
Contributor

SimenB commented Apr 7, 2017

This is still present in yarn 0.22.0.

Quick one-liner based on OP: yarn init -y && yarn add sequelize-cli && yarn add gulp --dev && yarn --prod && yarn check --verify-tree --prod still gives

yarn check v0.22.0
error "sequelize-cli#gulp" not installed
error Found 1 errors.

@bestander Could you please check this? There's also an open PR for this, I haven't tested it though #2921

The fact that I can't trust --prod erodes my trust in yarn, which is a shame, cuz it's really awesome in so many ways. For now we still install dev dependencies to production to avoid this, which bloats up the image, and slows down the link part of install into a docker container.

EDIT: I tested with 0.23.0, and it actually prints a warning during yarn --prod (#3025). It still fails the check, though.

yarn install v0.23.0
[1/4] Resolving packages...
warning Integrity check: Patterns don't match
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
Done in 2.69s.

Should the yarn --prod command fail at that point?

@bestander
Copy link
Member

Yeah, looks like the bug is present there.
warning Integrity check: Patterns don't match means that quick integrity check does not pass and install should proceed.

I'll investigate now.

@bestander
Copy link
Member

Apparently the logic here is incorrect https://github.com/yarnpkg/yarn/blob/master/src/package-hoister.js#L147 and does not factor top level devDependencies.
Anyone wants to dig deeper and send a PR?

@blexrob
Copy link
Contributor

blexrob commented Apr 7, 2017

@bestander - already did that, see #2921

@bestander
Copy link
Member

Awesome, thanks, @blexrob, sorry for missing this.

bestander pushed a commit that referenced this issue Apr 7, 2017
…evDependencies list (#2921)

* Test for install skipping subdependencies when those are named in root devDependencies

* Add 'incompatible' flag to references, use that to ignore incompatible packages instead of faulty inherit logic. Fixes #2819

* Unconditionally mark packages as ignored, hoister now fully corrects transitive uses
bestander pushed a commit that referenced this issue Apr 7, 2017
…evDependencies list (#2921) (#3065)

* Test for install skipping subdependencies when those are named in root devDependencies

* Add 'incompatible' flag to references, use that to ignore incompatible packages instead of faulty inherit logic. Fixes #2819

* Unconditionally mark packages as ignored, hoister now fully corrects transitive uses
arcanis pushed a commit to arcanis/yarn that referenced this issue Apr 7, 2017
…evDependencies list (yarnpkg#2921)

* Test for install skipping subdependencies when those are named in root devDependencies

* Add 'incompatible' flag to references, use that to ignore incompatible packages instead of faulty inherit logic. Fixes yarnpkg#2819

* Unconditionally mark packages as ignored, hoister now fully corrects transitive uses
bestander pushed a commit that referenced this issue Apr 7, 2017
* Checks that the webpack builds are working properly (#3064)

* Fix missing subdeps in production install when those are present in devDependencies list (#2921)

* Test for install skipping subdependencies when those are named in root devDependencies

* Add 'incompatible' flag to references, use that to ignore incompatible packages instead of faulty inherit logic. Fixes #2819

* Unconditionally mark packages as ignored, hoister now fully corrects transitive uses

* replaced deprecated asserts (#3069)

* fixing lint (#3070)

* Fixes integrity check for --production flag (#3067)

* fixed integrity check when running with --production

* added test

* removed unused var

* Remove the dependency on the "rc" module (#3063)

* Removes dependency on the "rc" module

* Removers shebang-loader, not used anymore

* Fixes flow errors

* Fixes tests on Windows
@ecbrodie
Copy link

ecbrodie commented May 2, 2017

Just a comment from my point of view, regarding this warning message (as a response to @bestander's comment):

warning Integrity check: Patterns don't match

When I see this after running a yarn install command, I begin to worry that there is either a yarn bug or an issue with one of our yarn files (such as yarn.lock). But rather, hearing that this message signifies a normal use case for yarn (in this case, I am running yarn install because I have not yet installed packages due to a recent yarn.lock or package.json change) is a little confusing. I would expect to receive silent output or non-warning output in the case of an expected use case.

If there is a better medium to provide this feedback (new Github issue?), then please let me know.

@SimenB
Copy link
Contributor

SimenB commented May 2, 2017

@ecbrodie That's fixed in #3248

@ecbrodie
Copy link

ecbrodie commented May 2, 2017

@SimenB excellent, thanks for letting me know. Which yarn version will this be released in?

@SimenB
Copy link
Contributor

SimenB commented May 2, 2017

No idea 😄

@bestander
Copy link
Member

It is in master branch right now
screen shot 2017-05-02 at 5 01 37 pm

So will be released in next branch cut, i.e. 0.24 this Thursday

@artlogic
Copy link

I just experienced this issue again using v0.24.5. Has there been a regression?

@bestander
Copy link
Member

bestander commented May 19, 2017 via email

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