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

graphql@16 conflicts with [email protected] peerDependencies, breaking dist build #2773

Closed
trentm opened this issue Jun 14, 2022 · 1 comment · Fixed by #2774
Closed

graphql@16 conflicts with [email protected] peerDependencies, breaking dist build #2773

trentm opened this issue Jun 14, 2022 · 1 comment · Fixed by #2774
Assignees
Labels
agent-nodejs Make available for APM Agents project planning.

Comments

@trentm
Copy link
Member

trentm commented Jun 14, 2022

Way back in #1840 we added support for node v15/v16, which involved npm v7. The npm v7 update results in a change in default npm behaviour handling peerDependencies: peer dep conflicts result in install errors. We have peer dep conflicts in our devDependencies from the challenges of express-graphql, graphql, and apollo-server-express.

At the time we worked around all this with a ".npmrc" file:

% cat .npmrc
# Workaround unresolvable peerDependencies between express-graphql, graphql,
# and apollo-server-express. npm v7 (included with node v15) makes these
# peerDependencies issues an install error. Until the community catches up
# and resolves peerDependencies issues or apm-agent-nodejs.git's tests are
# setup to not have competing deps in "devDependencies", we revert to the
# pre-v7 behavior.
# https://docs.npmjs.com/cli/v7/using-npm/config#legacy-peer-deps
legacy-peer-deps=true

However, the recent update (#2761) to graphql@16 in our devDependencies results in a new and related issue:

First, npm ls graphql fails:

% npm ls graphql
[email protected] /Users/trentm/el/apm-agent-nodejs
├─┬ [email protected]
│ ├─┬ @apollo/[email protected]
│ │ ├─┬ @apollo/[email protected]
│ │ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ │ ├─┬ @apollo/[email protected]
│ │ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ │ ├─┬ @apollo/[email protected]
│ │ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ │ ├─┬ @apollo/[email protected]
│ │ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ │ ├─┬ @apollo/[email protected]
│ │ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ ├─┬ @apollographql/[email protected]
│ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ ├─┬ @graphql-tools/[email protected]
│ │ ├─┬ @graphql-tools/[email protected]
│ │ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ ├─┬ @graphql-tools/[email protected]
│ │ ├─┬ @graphql-tools/[email protected]
│ │ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ ├─┬ [email protected]
│ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ ├─┬ [email protected]
│ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ ├─┬ [email protected]
│ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ ├─┬ [email protected]
│ │ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
│ └── [email protected] deduped
├─┬ [email protected]
│ └── [email protected] deduped
├─┬ [email protected]
│ └── [email protected] deduped invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql
└── [email protected] invalid: "^14.7.0 || ^15.3.0" from node_modules/express-graphql

npm ERR! code ELSPROBLEMS
npm ERR! invalid: [email protected] /Users/trentm/el/apm-agent-nodejs/node_modules/graphql

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/trentm/.npm/_logs/2022-06-14T15_42_01_040Z-debug-0.log

It is a little frustrating to me that neither npm ls or npm ci show any issues here. This npm ls graphql error shouldn't be fatal for us... at least I don't think so.

However this does break our dist build:

% make -C .ci dist BRANCH_NAME=asdf
../dev-utils/make-distribution.sh
elastic-apm-node-3.35.0.tgz
npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR!
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/graphql
npm ERR!   dev graphql@"^16.5.0" from the root project
npm ERR!   peer graphql@"^14.2.1 || ^15.0.0 || ^16.0.0" from @apollographql/[email protected]
npm ERR!   node_modules/@apollographql/apollo-tools
npm ERR!     @apollographql/apollo-tools@"^0.5.3" from [email protected]
npm ERR!     node_modules/apollo-server-core
npm ERR!       dev apollo-server-core@"^3.0.0" from the root project
npm ERR!       1 more (apollo-server-express)
npm ERR!   10 more (@graphql-tools/merge, @graphql-tools/mock, ...)
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer graphql@"^14.7.0 || ^15.3.0" from [email protected]
npm ERR! node_modules/express-graphql
npm ERR!   dev express-graphql@"^0.12.0" from the root project
npm ERR!
npm ERR! Conflicting peer dependency: [email protected]
npm ERR! node_modules/graphql
npm ERR!   peer graphql@"^14.7.0 || ^15.3.0" from [email protected]
npm ERR!   node_modules/express-graphql
npm ERR!     dev express-graphql@"^0.12.0" 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/trentm/.npm/eresolve-report.txt for a full report.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/trentm/.npm/_logs/2022-06-14T15_51_53_365Z-debug-0.log
make: *** [dist] Error 1

The reason this is happening, is because the way that "make-distribution.sh" sets up its clean build area to run npm ci --omit=dev --ignore-scripts:

mkdir -p nodejs/node_modules/elastic-apm-node
(cd nodejs/node_modules/elastic-apm-node;
# Use 'npm pack' to get the published files as a start.
npm --loglevel=warn pack "$TOP"; # creates "elastic-apm-node-$ver.tgz"
tar --strip-components=1 -xf elastic-apm-node-*.tgz;
rm elastic-apm-node-*.tgz;
cp $TOP/package-lock.json ./;
# Then install the "package-lock.json"-dictated dependencies (excluding
# devDependencies). Use '--ignore-scripts' to have confidence no code but
# ours and npm's is running.
npm ci --omit=dev --ignore-scripts;

results in it not having the .npmrc workaround that we have at the top-level.

Again, it is frustrating to me that even with the --omit=dev argument to npm ci we still get hit by errors with devDependencies.

options

  • Option A: resolve the peer dep conflicts somehow. This is a bigger chunk of work that won't get done now. (Likely the solution will be having all the testing deps be in a separate subdirs to test each instrumentation. This will separate the conflicting graphql-related modules.)
  • Option B: Get that .npmrc file carried through in make-distribution.sh's handling of the dist build.
  • Option C: Move our "graphql" dep in package.json back to 15.8.0 to satisfy the peerDependency in "express-graphql". Doing so will mean our regular test runs will be testing not the latest graphql version, and dependabot will offer an update we can't yet take. However, our TAV tests will still be testing graphql@16, so there is no big harm.

I don't have a strong preference between options B and C. Option A is the eventual right answer, but isn't going to happen right now.

@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Jun 14, 2022
@trentm trentm self-assigned this Jun 14, 2022
@trentm
Copy link
Member Author

trentm commented Jun 14, 2022

Option B doesn't solve that npm ls graphql is "broken" at the top-level, so I slightly prefer Option C for now.

trentm added a commit that referenced this issue Jun 14, 2022
This peer dep conflict breaks our dist build (make -C .ci dist).
Fix the conflict by backing up to graphql@15 for now -- until we
separate out testing deps to separate package dirs, or until
express-graphql updates to support graphql@16.
We still test graphql@16 with our TAV tests.

Fixes: #2773
trentm added a commit that referenced this issue Jun 14, 2022
…2774)

This peer dep conflict breaks our dist build (make -C .ci dist).
Fix the conflict by backing up to graphql@15 for now -- until we
separate out testing deps to separate package dirs, or until
express-graphql updates to support graphql@16.
We still test graphql@16 with our TAV tests.

Fixes: #2773
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant