Broken dependency to graphql - outdated apollo modules

[fastman]

Hi, It looks like there is a problem with graphql dependency. Some modules requires (as a peer dependency) an older version of graphql that is defined as a direct dependency of mockttp.

Test case (note UNMET PEER DEPENDENCY error)

$ docker run -it node:6 /bin/bash
root@5511b703a053:/# mkdir test_mockttp
root@5511b703a053:/# cd test_mockttp/
root@5511b703a053:/test_mockttp# npm init -y .
Wrote to /test_mockttp/package.json:

{
  "name": "test_mockttp",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC"
}

root@5511b703a053:/test_mockttp# npm install --save-dev mockttp

> core-js@2.6.9 postinstall /test_mockttp/node_modules/core-js
> node scripts/postinstall || echo "ignore"

test_mockttp@1.0.0 /test_mockttp
`-- mockttp@0.16.1
  +-- @types/cors@2.8.5
  +-- @types/express@4.17.0
  | +-- @types/body-parser@1.17.0
  | | `-- @types/connect@3.4.32
  | +-- @types/express-serve-static-core@4.16.7
  | | `-- @types/range-parser@1.2.3
  | `-- @types/serve-static@1.13.2
  |   `-- @types/mime@2.0.1
  +-- @types/node@10.14.13
  +-- @types/node-forge@0.8.4
  +-- apollo-server-express@1.4.0
  | +-- apollo-server-core@1.4.0
  | | +-- apollo-cache-control@0.1.1
  | | | `-- UNMET PEER DEPENDENCY graphql@0.10.x - 0.13.x
  | | +-- apollo-tracing@0.1.4
  | | | `-- UNMET PEER DEPENDENCY graphql@0.10.x - 0.13.x
  | | +-- UNMET PEER DEPENDENCY graphql@0.10.x - 0.13.x
  | | `-- graphql-extensions@0.0.10
  | |   +-- core-js@2.6.9
  | |   `-- source-map-support@0.5.12
  | |     +-- buffer-from@1.1.1
  | |     `-- source-map@0.6.1
  | +-- apollo-server-module-graphiql@1.4.0
  | `-- UNMET PEER DEPENDENCY graphql@^0.9.0 || ^0.10.0 || ^0.11.0 || ^0.12.0 || ^0.13.0
  +-- base64-arraybuffer@0.1.5
  +-- body-parser@1.19.0
  | +-- bytes@3.1.0
  | +-- content-type@1.0.4
  | +-- debug@2.6.9
  | | `-- ms@2.0.0
  | +-- depd@1.1.2
  | +-- http-errors@1.7.2
  | | `-- toidentifier@1.0.0
  | +-- iconv-lite@0.4.24
  | | `-- safer-buffer@2.1.2
  | +-- on-finished@2.3.0
  | | `-- ee-first@1.1.1
  | +-- qs@6.7.0
  | +-- raw-body@2.4.0
  | | `-- unpipe@1.0.0
  | `-- type-is@1.6.18
  |   +-- media-typer@0.3.0
  |   `-- mime-types@2.1.24
  |     `-- mime-db@1.40.0
  +-- brotli@1.3.2
  | `-- base64-js@1.3.0
  +-- common-tags@1.8.0
  +-- cors@2.8.5
  | +-- object-assign@4.1.1
  | `-- vary@1.1.2
  +-- express@4.17.1
  | +-- accepts@1.3.7
  | | `-- negotiator@0.6.2
  | +-- array-flatten@1.1.1
  | +-- content-disposition@0.5.3
  | +-- cookie@0.4.0
  | +-- cookie-signature@1.0.6
  | +-- encodeurl@1.0.2
  | +-- escape-html@1.0.3
  | +-- etag@1.8.1
  | +-- finalhandler@1.1.2
  | +-- fresh@0.5.2
  | +-- merge-descriptors@1.0.1
  | +-- methods@1.1.2
  | +-- parseurl@1.3.3
  | +-- path-to-regexp@0.1.7
  | +-- proxy-addr@2.0.5
  | | +-- forwarded@0.1.2
  | | `-- ipaddr.js@1.9.0
  | +-- range-parser@1.2.1
  | +-- safe-buffer@5.1.2
  | +-- send@0.17.1
  | | +-- destroy@1.0.4
  | | +-- mime@1.6.0
  | | `-- ms@2.1.1
  | +-- serve-static@1.14.1
  | +-- setprototypeof@1.1.1
  | +-- statuses@1.5.0
  | `-- utils-merge@1.0.1
  +-- fetch-ponyfill@4.1.0
  | `-- node-fetch@1.7.3
  |   +-- encoding@0.1.12
  |   `-- is-stream@1.1.0
  +-- UNMET PEER DEPENDENCY graphql@14.4.2
  | `-- iterall@1.2.2
  +-- graphql-subscriptions@0.5.8
  +-- graphql-tools@2.18.0
  | +-- apollo-link@1.2.12
  | | +-- ts-invariant@0.4.4
  | | +-- tslib@1.10.0
  | | `-- zen-observable-ts@0.8.19
  | |   `-- zen-observable@0.8.14
  | +-- apollo-utilities@1.3.2
  | | +-- @wry/equality@0.1.9
  | | `-- fast-json-stable-stringify@2.0.0
  | +-- deprecated-decorator@0.1.6
  | `-- UNMET PEER DEPENDENCY graphql@^0.10.5 || ^0.11.3 || ^0.12.0 || ^0.13.0
  +-- httpolyglot@0.1.2
  +-- lodash@4.17.15
  +-- native-duplexpair@1.0.0
  +-- node-forge@0.8.5
  +-- normalize-url@1.9.1
  | +-- prepend-http@1.0.4
  | +-- query-string@4.3.4
  | | `-- strict-uri-encode@1.1.0
  | `-- sort-keys@1.1.2
  |   `-- is-plain-obj@1.1.0
  +-- performance-now@2.1.0
  +-- portfinder@1.0.21
  | +-- async@1.5.2
  | `-- mkdirp@0.5.1
  |   `-- minimist@0.0.8
  +-- subscriptions-transport-ws@0.9.16
  | +-- backo2@1.0.2
  | +-- eventemitter3@3.1.2
  | +-- symbol-observable@1.2.0
  | `-- ws@5.2.2
  |   `-- async-limiter@1.0.0
  +-- typed-error@3.1.0
  +-- universal-websocket-client@1.0.2
  | `-- ws@3.3.3
  |   `-- ultron@1.1.1
  +-- uuid@3.3.2
  `-- websocket-stream@5.5.0
    +-- duplexify@3.7.1
    | +-- end-of-stream@1.4.1
    | | `-- once@1.4.0
    | |   `-- wrappy@1.0.2
    | `-- stream-shift@1.0.0
    +-- inherits@2.0.3
    +-- readable-stream@2.3.6
    | +-- core-util-is@1.0.2
    | +-- isarray@1.0.0
    | +-- process-nextick-args@2.0.1
    | +-- string_decoder@1.1.1
    | `-- util-deprecate@1.0.2
    +-- ws@3.3.3
    `-- xtend@4.0.2

npm WARN graphql-subscriptions@0.5.8 requires a peer of graphql@^0.10.5 || ^0.11.3 || ^0.12.0 || ^0.13.0 but none was installed.
npm WARN graphql-tools@2.18.0 requires a peer of graphql@^0.11.0 || ^0.12.0 but none was installed.
npm WARN apollo-server-core@1.4.0 requires a peer of graphql@^0.9.0 || ^0.10.0 || ^0.11.0 || ^0.12.0 || ^0.13.0 but none was installed.
npm WARN apollo-tracing@0.1.4 requires a peer of graphql@0.10.x - 0.13.x but none was installed.
npm WARN apollo-cache-control@0.1.1 requires a peer of graphql@0.10.x - 0.13.x but none was installed.
npm WARN graphql-extensions@0.0.10 requires a peer of graphql@0.10.x - 0.13.x but none was installed.
npm WARN test_mockttp@1.0.0 No description
npm WARN test_mockttp@1.0.0 No repository field.
root@5511b703a053:/test_mockttp# node -v
v6.17.1
root@5511b703a053:/test_mockttp# npm --versions
{ test_mockttp: '1.0.0',
  npm: '3.10.10',
  ares: '1.10.1-DEV',
  http_parser: '2.8.0',
  icu: '58.2',
  modules: '48',
  napi: '3',
  node: '6.17.1',
  openssl: '1.0.2r',
  uv: '1.16.1',
  v8: '5.1.281.111',
  zlib: '1.2.11' } 

[pimterry]

Hi @fastman: you're totally right. I'm not totally sure what's messed up in the web of graphql dependencies, but the Apollo & graphql system is a bit of a mess there, and it needs sorting out.

In practice though as an end user you should be to ignore this. I'd like to fix the warning, but mockttp's dependencies should install & work fine regardless.

Of course do let me know if you are hitting concrete issues that might be caused by this (e.g. mockttp errors if other packages are installed), but I haven't seen anything like that myself, just the annoying warning.

[fastman]

@pimterry Eg. one of the issue is that apollo-cache-control@0.1.1 requires graphql to be one of the version between 0.10.x - 0.13.x but the version ^14.0.2 is installed as a dependency to mockttp. The same goes for other cases. Two solutions: 1) apollo-* modules get updated 2) graphql version in mockttp is specified to match those peer dependencies requirements from apollo-*

[pimterry]

Yes, I've tried to update some of these in the past. I believe it's a little more complicated than you'd expect though, as updates through some breaking changes are required, so it needs a little work & testing, and comes with some risk. It's definitely planned though, it's just not an urgent priority, as far as I'm aware.

Is this causing issues in your application?

[fastman]

Yeah, I'm perfectly aware of what a pain it would be to update those dependencies.

The problem is that it's impossible to use shrinkwrap/lock file to reliably reproduce the environment, And the workaround is not too good - you have to add "additional" version of graphql to match apollo-* requirements, even if you don't use graphql at all. You end up with something like this:

{
  "name": "test_mockttp",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "express": "^4.17.1"
  },
  "devDependencies": {
    "graphql": "^0.12.3",
    "mockttp": "^0.16.1",
    "react": "^16.8.6"
  }
}

(ofc, react and express are just placeholders here)

[pimterry]

The problem is that it's impossible to use shrinkwrap/lock file to reliably reproduce the environment

Is that true? Why?

As far as I'm aware, these warnings shouldn't affect lock files at all, I believe that peer dependencies only serve to provide these install warnings nowadays. As a quick test:

• npm init -y
• npm install mockttp: prints this warning, but installs fine
• npm ci: cleans & reinstalls happily from the lockfile, returns all good

npm ci will fail if the lockfile isn't sufficient to exactly reproduce the environment, so should be a reasonable way to test this.

Does that not match the behaviour you're seeing? Is there a way to directly reproduce your issue from the package.json above? I've tested it with & without the extra graphql dependency, and the only difference I can see is in the warnings on the initial npm install. In both cases it looks to me like the lockfile is generated fine afterwards, and works as expected with npm install and npm ci.

[fastman]

@pimterry fair enough - works fine with npm 6.4.1 and 6.9.0 (bundled with node 8 and 10 respectively). Npm still complains about missing peer dependencies but at least shrinkwrap file is updated when adding mockttp.

[pimterry]

I've now fixed 1/3 of these (https://github.com/httptoolkit/mockttp/commit/4c074abf3086e3ec80154b4c10ec77bed58866d3), since they were quick and easy. The remaining 2/3s are harder, and all come from the dependency on apollo-server-express v1. Upgrading to v2 is non-trivial, and would increase the size of this package by nearly 30% just to get rid of these warnings, which is pretty annoying.

I'll investigate the rest at a later date. I'm likely to either dump Apollo and move to express-graphql, or maybe dump graphql entirely. When I initially set it up I had a different vision of querying the collected data, and I don't think it's necessary at all now, it's just extra weight and complexity.

I'm not going to do that in the short term, but medium term: watch this space.

[pimterry]

As part of removing Apollo entirely, this is now 100% fixed on master :+1: