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

npm updates are not shallow #2348

Open
felixfbecker opened this Issue Aug 5, 2018 · 6 comments

Comments

Projects
None yet
3 participants
@felixfbecker

felixfbecker commented Aug 5, 2018

What Renovate type are you using?
Renovate GitHub App

Describe the bug
This is npm behaviour, but I wanted to post it here to raise awareness and maybe see if Renovate can work around it, as it is especially unexpected for Renovate.

When updating a dependency pkg, Renovate will update all the dependencies of pkg to the latest version too, even though the expectation is that every package update is isolated. This brought us a bad bug in a transitive dependency recently when we thought we were just updating one of our private packages to include a tiny bug fix, but in reality it also updated the indirect dependencies (which is even hidden in the diff as package-lock gets collapsed by GitHub). This is especially bad when using automerge in certain packageRules, because the packageRule actually only applies to the top-level dependency, but indirect dependencies may be updated & automerged too.

I filed this on npm with additional details and repro steps: https://npm.community/t/impossible-to-update-single-package-without-updating-its-dependencies/1156

But knowing how busy the npm team is, I wonder if Renovate could work around this somehow, e.g. manually editing the lockfile to ensure indirect dependencies stay the same.

@rarkins

This comment has been minimized.

Show comment
Hide comment
@rarkins

rarkins Aug 5, 2018

Collaborator

I’m curious: is the “deep” behaviour exactly the same if pkg-a was pinned to 1.0.0 first? Ie upgrade is 1.0.0 -> 1.0.1 instead of ^1.0.0 -> ^1.0.1?

I agree it would be nice for npm to have the concept of a “shallow” update that updates the minimum possible transitive dependencies but I am concerned it could introduce a lot of complexity and risk if Renovate were to start second-guessing npm’s resolution algorithm.

Something like “if the new version’s dependencies are identical to the old version’s dependencies then update the lock file manually instead of calling npm install” might be feasible though. Ie instead of calling out to npm before then trying to “reverse” some of npm’s changes.

Also: even if we did this, it would be for plain repos only and not lerna, because for lerna monorepos we update lockfiles via lerna not npm.

Collaborator

rarkins commented Aug 5, 2018

I’m curious: is the “deep” behaviour exactly the same if pkg-a was pinned to 1.0.0 first? Ie upgrade is 1.0.0 -> 1.0.1 instead of ^1.0.0 -> ^1.0.1?

I agree it would be nice for npm to have the concept of a “shallow” update that updates the minimum possible transitive dependencies but I am concerned it could introduce a lot of complexity and risk if Renovate were to start second-guessing npm’s resolution algorithm.

Something like “if the new version’s dependencies are identical to the old version’s dependencies then update the lock file manually instead of calling npm install” might be feasible though. Ie instead of calling out to npm before then trying to “reverse” some of npm’s changes.

Also: even if we did this, it would be for plain repos only and not lerna, because for lerna monorepos we update lockfiles via lerna not npm.

@rarkins

This comment has been minimized.

Show comment
Hide comment
@rarkins

rarkins Aug 5, 2018

Collaborator

One more thing: you would have an increased chance of catching the problem as-is if Renovate included a summary in the PR body of which packages are changed (including transitive).

Collaborator

rarkins commented Aug 5, 2018

One more thing: you would have an increased chance of catching the problem as-is if Renovate included a summary in the PR body of which packages are changed (including transitive).

@hbetts

This comment has been minimized.

Show comment
Hide comment
@hbetts

hbetts Aug 5, 2018

Collaborator

I agree it would be nice for npm to have the concept of a “shallow” update that updates the minimum possible transitive dependencies

Sounds very much like Minimum Version Selection in vgo.

update the lock file manually instead of calling npm install

Just my two cents, but to me, that sounds like a really bad idea.

Also, I believe you would loose de-duplication if the manually upgraded version matched a similar requirement by another package.

if Renovate included a summary in the PR body of which packages are changed

Again, my two cents, but I view transitive dependencies as more of an implementation detail of my direct dependencies. I only care about my direct dependency's API, not how it's implemented.

Collaborator

hbetts commented Aug 5, 2018

I agree it would be nice for npm to have the concept of a “shallow” update that updates the minimum possible transitive dependencies

Sounds very much like Minimum Version Selection in vgo.

update the lock file manually instead of calling npm install

Just my two cents, but to me, that sounds like a really bad idea.

Also, I believe you would loose de-duplication if the manually upgraded version matched a similar requirement by another package.

if Renovate included a summary in the PR body of which packages are changed

Again, my two cents, but I view transitive dependencies as more of an implementation detail of my direct dependencies. I only care about my direct dependency's API, not how it's implemented.

@rarkins rarkins changed the title from Renovate updates are not shallow to npm updates are not shallow Aug 6, 2018

@felixfbecker

This comment has been minimized.

Show comment
Hide comment
@felixfbecker

felixfbecker Sep 1, 2018

I wrote an RFC for npm: npm/rfcs#21

felixfbecker commented Sep 1, 2018

I wrote an RFC for npm: npm/rfcs#21

@rarkins

This comment has been minimized.

Show comment
Hide comment
@rarkins

rarkins Oct 7, 2018

Collaborator

@felixfbecker you mentioned in #2294 (comment) that you had switched to Yarn. Have you had time to check how Yarn handles this same "shallow" problem?

Collaborator

rarkins commented Oct 7, 2018

@felixfbecker you mentioned in #2294 (comment) that you had switched to Yarn. Have you had time to check how Yarn handles this same "shallow" problem?

@felixfbecker

This comment has been minimized.

Show comment
Hide comment
@felixfbecker

felixfbecker Oct 7, 2018

Yes, yarn has the same problem: yarnpkg/yarn#5475

felixfbecker commented Oct 7, 2018

Yes, yarn has the same problem: yarnpkg/yarn#5475

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