dependabot / dependabot-core

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

Update to jest does not pull in required update to ts-jest #5922

Open sjlehn opened 2 years ago

sjlehn commented 2 years ago

Is there an existing issue for this?

Package ecosystem

npm

Package manager version

npm 6.14.17

Language version

nodejs v14.20.1

Manifest location and content before the Dependabot update

package.json:

{
  "name": "api",
  "version": "0.0.0",
  "description": "",
  "main": "build/src/index.js",
  "types": "build/src/index.d.ts",
  "license": "UNLICENSED",
  "keywords": [],
  "scripts": {
    "start": "nodemon --exec 'node --require ts-node/register --inspect=0.0.0.0 src/index.ts' --ignore \"features/*\" --ignore \"build/*\"",
    "lint": "gts lint",
    "clean": "gts clean",
    "compile": "tsc --build tsconfig.prod.json",
    "fix": "gts fix",
    "test": "jest && node_modules/.bin/ts-node node_modules/.bin/cucumber-js --require \"features/support/*.ts\"",
    "test:unit": "jest --testNamePattern \"(?!.*(mysql))^.*$\"",
    "test:integration": "jest --testNamePattern mysql",
    "test:e2e": "node_modules/.bin/ts-node node_modules/.bin/cucumber-js --require \"features/support/*.ts\""
  },
  "devDependencies": {
    "@cucumber/cucumber": "^7.3.1",
    "@types/body-parser": "^1.19.2",
    "@types/cors": "^2.8.12",
    "@types/express": "^4.17.13",
    "@types/jest": "^27.0.2",
    "@types/morgan": "^1.9.3",
    "@types/mysql": "^2.15.19",
    "@types/node": "^14.17.5",
    "@types/swagger-ui-express": "^4.1.3",
    "@types/yamljs": "^0.2.31",
    "gts": "^3.1.1",
    "jest": "^26.6.3",
    "nodemon": "^2.0.20",
    "ts-jest": "^26.5.6",
    "ts-node": "^10.7.0",
    "typescript": "^4.3.5"
  },
  "dependencies": {
    "@aws-cdk/aws-ecs": "^1.176.0",
    "@types/passport": "^1.0.11",
    "@types/passport-http": "^0.3.9",
    "body-parser": "^1.20.0",
    "cors": "^2.8.5",
    "express": "^4.17.1",
    "jsonschema": "^1.4.0",
    "morgan": "^1.10.0",
    "mysql": "^2.18.1",
    "passport": "^0.6.0",
    "passport-http": "^0.3.0",
    "swagger-ui-express": "^4.5.0",
    "yamljs": "^0.3.0"
  },
  "engines": {
    "node": ">=9.0.0"
  }
}

dependabot.yml content

version: 2
updates:
  - package-ecosystem: "npm"
    directory: "/auth0-api"
    schedule:
      interval: "daily"
    open-pull-requests-limit: 2
    rebase-strategy: "disabled"

Updated dependency

Updates jest from 26.6.3 to 29.2.0 Updates @types/jest from 27.0.2 to 29.1.2

Should update ts-jest from 26.5.6 to 29.0.3

What you expected to see, versus what you actually saw

A PR was opened to update jest and @types/jest, but ts-jest was not included. This resulted in a failure during the CI run:

Test Suites: 0 of 4 total
Tests:       0 total
Snapshots:   0 total
Time:        0.226 s
Ran all test suites.
Error: ● Invalid transformer module:
  "/app/node_modules/ts-jest/dist/index.js" specified in the "transform" object of Jest configuration
  must export a `process` or `processAsync` or `createTransformer` function.
  Code Transformation Documentation:
  https://jestjs.io/docs/code-transformation

    at /app/node_modules/@jest/transform/build/ScriptTransformer.js:315:19
    at async Promise.all (index 0)
    at async ScriptTransformer.loadTransformers (/app/node_modules/@jest/transform/build/ScriptTransformer.js:308:5)
    at async createScriptTransformer (/app/node_modules/@jest/transform/build/ScriptTransformer.js:977:3)
    at async /app/node_modules/@jest/core/build/TestScheduler.js:221:33
    at async Promise.all (index 0)
    at async TestScheduler.scheduleTests (/app/node_modules/@jest/core/build/TestScheduler.js:217:7)
    at async runJest (/app/node_modules/@jest/core/build/runJest.js:349:19)
    at async _run10000 (/app/node_modules/@jest/core/build/cli/index.js:326:7)
    at async runCLI (/app/node_modules/@jest/core/build/cli/index.js:191:3)
npm ERR! Test failed.  See above for more details.

Manually updating ts-jest resolved the error.

Native package manager behavior

No response

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

No response

Smallest manifest that reproduces the issue

No response

jeffwidman commented 1 year ago

👋 apologies, I'm not super familiar with the npm ecosystem.

How does native npm realize that ts-jest is associated with jest/@types/jest? Where is that mapping specified? Is it in the package.json of one of the underlying packages?

sjlehn commented 1 year ago

I'm not super familiar with the intricacies of npm, but from what I can tell, ts-jest specifies a peer dependency on jest in its package.json

jeffwidman commented 1 year ago

Right, so that makes sense that if updating ts-jest that Dependabot needs to also bump jest/@types/jest...

But if bumping only jest/@types/jest, unless they specify ts-jest as a peer dependency, then how would npm (and thereby Dependabot) realize it needs to bump ts-jest? I guess if we're running npm against the whole thing and not individual packages, then it should complain when it realizes ts-jest is no longer compatible with the proposed bump...

So what did running native npm do here when trying to bump only jest/@types/jest? Did it also bump ts-jest, or complain, or let the update proceed w/o bumping ts-jest?

I'll ask a colleague who knows more about npm to take a peek at this...

jeffwidman commented 1 year ago

Also, what did running native npm do here when trying to bump only jest/@types/jest? Did it also bump ts-jest, or complain, or let the update proceed w/o bumping ts-jest?

sjlehn commented 1 year ago

I don't recall exactly, I'd have to recreate the environment and play with it a bit. Updating jest without ts-jest didn't cause npm to fail (it may have emitted a warning), as we ended up getting the errors farther on when we actually tried to use jest. I can try to refresh my memory on this on Monday if somebody hasn't already figured it out.

bdragon commented 1 year ago

We added the capability to automatically update the corresponding @types/ package when updating a dependency last year. AFAIK this doesn't extend to peerDependencies. Still, even though jest is a peerDependency of ts-jest, the reverse is not true.

sjlehn commented 1 year ago

even though jest is a peerDependency of ts-jest, the reverse is not true.

which would imply that there's no good way to pick up this sort of error?

I just tried the following:

added 130 packages, removed 226 packages, changed 60 packages, and audited 848 packages in 5s

71 packages are looking for funding run npm fund for details

found 0 vulnerabilities


Try to upgrade on Node 14 is similar:

npm notice created a lockfile as package-lock.json. You should commit this file. npm WARN ts-jest@26.5.6 requires a peer of jest@>=26 <27 but none is installed. You must install peer dependencies yourself. npm WARN api@0.0.0 No description npm WARN api@0.0.0 No repository field.

added 769 packages from 638 contributors and audited 772 packages in 19.608s

71 packages are looking for funding run npm fund for details

found 0 vulnerabilities


Upgrading `ts-jest` with node 16 actually fails:

$ npm install ts-jest@29.0.3 ⬡ system npm ERR! code ERESOLVE npm ERR! ERESOLVE unable to resolve dependency tree npm ERR! npm ERR! While resolving: api@0.0.0 npm ERR! Found: jest@26.6.3 npm ERR! node_modules/jest npm ERR! dev jest@"^26.6.3" from the root project npm ERR! npm ERR! Could not resolve dependency: npm ERR! peer jest@"^29.0.0" from ts-jest@29.0.3 npm ERR! node_modules/ts-jest npm ERR! dev ts-jest@"29.0.3" from the root project npm ERR! npm ERR! Fix the upstream dependency conflict, or retry npm ERR! this command with --force, or --legacy-peer-deps npm ERR! to accept an incorrect (and potentially broken) dependency resolution. npm ERR! npm ERR! See /Users/stevenlehn/.npm/eresolve-report.txt for a full report.

npm ERR! A complete log of this run can be found in: npm ERR! /Users/stevenlehn/.npm/_logs/2023-02-03T21_55_26_730Z-debug-0.log


One last test, upgrading with node 14, this just emits a warning:

$ npm install ts-jest@29.0.3 ⬡ 14.20.1 npm WARN ts-jest@29.0.3 requires a peer of jest@^29.0.0 but none is installed. You must install peer dependencies yourself. npm WARN api@0.0.0 No description npm WARN api@0.0.0 No repository field.

73 packages are looking for funding run npm fund for details

found 0 vulnerabilities



Hopefully that's some help.
jeffwidman commented 1 year ago

So @bdragon did a little more digging, and the upshot was:

  1. the behavior of npm 6 is a warning, vs npm 7+ is an error. In Dependabot, we try to parse the npm 6 warning and error out, like npm 7/8 does.
  2. So this PR showing up at all is unexpected... the ts-jest peerDependency constraint should have resulted in an update_not_possible error, not sure why it didn't...
  3. Although currently Dependabot doesn't check peerDependencies for conflicts to try to see what else it needs to bump, that behavior could be extended later... that falls under the broader grouped updates category, which we do want to tackle at some point, but unclear what shape that would eventually land in.

So again, this does appear to be a bug that ts-jest peerDependency constraint wasn't accounted for, probably if it was accounted for it would have resulted in no update possible rather than a PR at all.

Unfortunately I can't look into this further right now, and I already nerdsniped _bdragon enough today, but if you want to step further with it it should be relatively straightforward to use docker and use the dry-run script to step through what's happening here... even if you don't know ruby, it's not hard to add some debugger + puts statements to step through what's happening. If you can narrow it down a bit, then myself or someone else might be able to help put together a fix.

sjlehn commented 1 year ago

"nerdsniped"...TIL. I hope I'm not getting myself in trouble for it. :) Maybe I can play with it a bit tomorrow and see what I can learn.

jeffwidman commented 1 year ago

If you want to play with this, the dry-run or CLI tool are both good options for simulating the job locally.