search

dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.7k stars 1.02k forks source link

Dependabot creates invalid update for npm due to "overriding peer dependency" #8248

Open ahilke opened 1 year ago

ahilke commented 1 year ago

Is there an existing issue for this?

Package ecosystem

npm

Package manager version

9.6.7

Language version

v18.17.1

Manifest location and content before the Dependabot update

./package.json

{
    "name": "crap-score",
    "version": "1.2.0",
    "description": "Calculate and visualize the CRAP score of a JS/TS project using the provided `jest` integration, CLI, or API.",
    "license": "MIT",
    "keywords": [
        "CRAP Score",
        "Cyclomatic Complexity",
        "Code Quality",
        "Test Coverage"
    ],
    "author": {
        "name": "Arno Hilke",
        "email": "arno.hilke@tum.de"
    },
    "repository": {
        "type": "git",
        "url": "https://github.com/ahilke/js-crap-score.git"
    },
    "bugs": {
        "url": "https://github.com/ahilke/js-crap-score/issues"
    },
    "type": "module",
    "main": "./dist/index.js",
    "exports": "./dist/index.js",
    "types": "./dist/index.d.ts",
    "bin": {
        "crap": "./dist/crap.js"
    },
    "scripts": {
        "build": "nest build",
        "prepublishOnly": "npm run build",
        "start": "nest start",
        "start:dev": "nest start --watch",
        "start:debug": "nest start --debug",
        "start:prod": "node ./dist/main.js",
        "test": "NODE_OPTIONS=--experimental-vm-modules NODE_NO_WARNINGS=1 jest",
        "test:dev": "npm run test -- --watch",
        "test:debug": "node --experimental-vm-modules --no-warnings --inspect ./node_modules/.bin/jest --runInBand",
        "test:cov": "npm run test -- --coverage",
        "test:generate-data": "npm run test -- --config=./test-data/esm/jest.config.ts && jest --config=./test-data/cjs/jest.config.ts",
        "prettier": "prettier --write ."
    },
    "engines": {
        "node": ">=18.17.1",
        "npm": ">=9.6.7"
    },
    "dependencies": {
        "@nestjs/cli": "^10.1.18",
        "@nestjs/common": "^10.2.7",
        "@nestjs/core": "^10.2.7",
        "@types/inquirer": "^8.2.9",
        "@typescript-eslint/parser": "^6.8.0",
        "@typescript-eslint/utils": "^6.8.0",
        "eslint": "^8.51.0",
        "handlebars": "^4.7.8",
        "lodash-es": "^4.17.21",
        "nest-commander": "^3.11.1",
        "reflect-metadata": "^0.1.13",
        "rxjs": "^7.8.1"
    },
    "devDependencies": {
        "@jest/reporters": "^29.7.0",
        "@jest/test-result": "^29.7.0",
        "@nestjs/testing": "^10.2.7",
        "@types/lodash-es": "^4.17.10",
        "prettier": "^3.0.3",
        "ts-jest": "^29.1.1",
        "ts-node": "^10.9.1",
        "typescript": "^5.2.2"
    }
}

https://github.com/ahilke/js-crap-score/blob/bf7a32705e23a1f7733493b7be9d4573ce3dfee2/package.json

dependabot.yml content

./.github/dependabot.yml

version: 2
updates:
    - package-ecosystem: "npm"
      directory: "/"
      schedule:
          interval: "weekly"

https://github.com/ahilke/js-crap-score/blob/bf7a32705e23a1f7733493b7be9d4573ce3dfee2/.github/dependabot.yml

Updated dependency

nest-commander from 3.11.1 to 3.12.0.

What you expected to see, versus what you actually saw

A PR was created to bump nest-commander from 3.11.1 to 3.12.0. This creates an invalid dependency tree. When checked out and running npm ci locally, it produces the following warnings:

% npm ci
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @fig/complete-commander@2.0.1
npm WARN Found: commander@11.0.0
npm WARN node_modules/nest-commander/node_modules/commander
npm WARN   commander@"11.0.0" from nest-commander@3.12.0
npm WARN   node_modules/nest-commander
npm WARN     nest-commander@"^3.11.1" from the root project
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer commander@"^8.0.0" from @fig/complete-commander@2.0.1
npm WARN node_modules/nest-commander/node_modules/@fig/complete-commander
npm WARN   @fig/complete-commander@"^2.0.1" from nest-commander@3.12.0
npm WARN   node_modules/nest-commander
npm WARN 
npm WARN Conflicting peer dependency: commander@8.3.0
npm WARN node_modules/commander
npm WARN   peer commander@"^8.0.0" from @fig/complete-commander@2.0.1
npm WARN   node_modules/nest-commander/node_modules/@fig/complete-commander
npm WARN     @fig/complete-commander@"^2.0.1" from nest-commander@3.12.0
npm WARN     node_modules/nest-commander

When running npm ls commander, it shows the invalid dependency resolution:

npm ls commander
npm ERR! code ELSPROBLEMS
npm ERR! invalid: commander@11.0.0 /Users/arnohilke/projects/js-crap-score/node_modules/nest-commander/node_modules/commander
crap-score@1.2.0 /Users/arnohilke/projects/js-crap-score
├─┬ @nestjs/cli@10.1.18
│ ├── commander@4.1.1
│ └─┬ webpack@5.88.2
│   └─┬ terser-webpack-plugin@5.3.9
│     └─┬ terser@5.19.2
│       └── commander@2.20.3
└─┬ nest-commander@3.12.0
  ├─┬ @fig/complete-commander@2.0.1
  │ └── commander@11.0.0 deduped invalid: "^8.0.0" from node_modules/nest-commander/node_modules/@fig/complete-commander
  └── commander@11.0.0 invalid: "^8.0.0" from node_modules/nest-commander/node_modules/@fig/complete-commander

While I'm aware there could be an issue with npm or the related packages producing the invalid dependency tree in the first place, I'd expect Dependabot to not create invalid updates or at least create some sort of warning. Since there is no way to bump nest-commander with a valid dependency tree, I expect no PR to be opened or at least some sort of warning within the PR.

Native package manager behavior

A manual update produce the same warning as above:

% npm install --save nest-commander@latest 
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @fig/complete-commander@2.0.1
npm WARN Found: commander@11.0.0
npm WARN node_modules/nest-commander/node_modules/commander
npm WARN   commander@"11.0.0" from nest-commander@3.12.0
npm WARN   node_modules/nest-commander
npm WARN     nest-commander@"3.12.0" from the root project
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer commander@"^8.0.0" from @fig/complete-commander@2.0.1
npm WARN node_modules/nest-commander/node_modules/@fig/complete-commander
npm WARN   @fig/complete-commander@"^2.0.1" from nest-commander@3.12.0
npm WARN   node_modules/nest-commander
npm WARN 
npm WARN Conflicting peer dependency: commander@8.3.0
npm WARN node_modules/commander
npm WARN   peer commander@"^8.0.0" from @fig/complete-commander@2.0.1
npm WARN   node_modules/nest-commander/node_modules/@fig/complete-commander
npm WARN     @fig/complete-commander@"^2.0.1" from nest-commander@3.12.0
npm WARN     node_modules/nest-commander

Images of the diff or a link to the PR, issue, or logs

https://github.com/ahilke/js-crap-score/pull/28

Smallest manifest that reproduces the issue

{
  "dependencies": {
    "@nestjs/cli": "10.1.18",
    "nest-commander": "3.11.1"
  }
}

Using node 18.7.1 and npm 9.6.7, add this to an empty folder and run npm i to create a valid dependency tree. Run then npm i nest-commander@3.12.0 to create the invalid dependency tree.

deivid-rodriguez commented 1 year ago

Hello!

Thanks for the report, this is a bug.

The problem is that dependabot only detects first level peer dependency conflicts, but in this case, the relevant conflict is happening at the second level:

npm WARN Could not resolve dependency:
npm WARN peer commander@"^8.0.0" from @fig/complete-commander@2.0.1
npm WARN node_modules/nest-commander/node_modules/@fig/complete-commander
npm WARN   @fig/complete-commander@"^2.0.1" from nest-commander@3.12.0
npm WARN   node_modules/nest-commander

It's mainly the following line that's relevant to us, but we don't parse it: 

npm WARN   @fig/complete-commander@"^2.0.1" from nest-commander@3.12.0

The relevant regex is here (and the code using it lives in that same file):

https://github.com/dependabot/dependabot-core/blob/be209fa2ad4f5c3cf6a76074ddf3b7a0247bcc87/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/version_resolver.rb#L69-L80

We should expand the logic so that we're able to scan all unmet peer dependencies, not just the first level.

jasonrichdarmawan commented 8 months ago

Can confirm. I just run this in Angular 17 repository, it caused conflicting peer dependency on 2 separate occassion.

eseidel commented 5 months ago

Curious if anyone has come up with a work-around? (e.g. maybe making one of these conflicts explicit/top-level to appease dependabot?) We hit this with eslint-parser across 4 of our repos, rendering dependabot useless for npm updates unfortunately.

roldengarm commented 2 months ago

Same problem here, very keen to hear a workaround or solution.

corneliusroemer commented 2 months ago

Same problem, in this case with eslint being bumped to v9 despite eslint-plugin-import not being v9-ready:

npm error code ERESOLVE
npm error ERESOLVE could not resolve
npm error
npm error While resolving: eslint-plugin-import@2.29.1
npm error Found: eslint@9.9.1
npm error node_modules/eslint
npm error   dev eslint@"^9.9.1" from the root project
npm error   peer eslint@"^6.0.0 || ^7.0.0 || >=8.0.0" from @eslint-community/eslint-utils@4.4.0
npm error   node_modules/@eslint-community/eslint-utils
npm error     @eslint-community/eslint-utils@"^4.4.0" from @typescript-eslint/utils@8.3.0
npm error     node_modules/@typescript-eslint/eslint-plugin/node_modules/@typescript-eslint/utils
npm error       @typescript-eslint/utils@"8.3.0" from @typescript-eslint/eslint-plugin@8.3.0
npm error       node_modules/@typescript-eslint/eslint-plugin
npm error         dev @typescript-eslint/eslint-plugin@"^8.3.0" from the root project
npm error     @eslint-community/eslint-utils@"^4.4.0" from @typescript-eslint/utils@8.3.0
npm error     node_modules/@typescript-eslint/type-utils/node_modules/@typescript-eslint/utils
npm error       @typescript-eslint/utils@"8.3.0" from @typescript-eslint/type-utils@8.3.0
npm error       node_modules/@typescript-eslint/type-utils
npm error         @typescript-eslint/type-utils@"8.3.0" from @typescript-eslint/eslint-plugin@8.3.0
npm error         node_modules/@typescript-eslint/eslint-plugin
npm error     3 more (@typescript-eslint/utils, eslint, eslint-plugin-astro)
npm error   10 more (@tanstack/eslint-plugin-query, ...)
npm error
npm error Could not resolve dependency:
npm error peer eslint@"^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8" from eslint-plugin-import@2.29.1
npm error node_modules/eslint-plugin-import
npm error   dev eslint-plugin-import@"^2.29.1" from the root project
npm error
npm error Conflicting peer dependency: eslint@8.57.0
npm error node_modules/eslint
npm error   peer eslint@"^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8" from eslint-plugin-import@2.29.1
npm error   node_modules/eslint-plugin-import
npm error     dev eslint-plugin-import@"^2.29.1" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution.

from github.com/loculus-project/loculus/pull/2583