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

node-fetch runtime dependency is missing in @apollo/gateway v2.0.5 #1961

Closed
marcelkottmann opened this issue Jul 6, 2022 · 3 comments · Fixed by #1970
Closed

node-fetch runtime dependency is missing in @apollo/gateway v2.0.5 #1961

marcelkottmann opened this issue Jul 6, 2022 · 3 comments · Fixed by #1970
Assignees
Labels
Milestone

Comments

@marcelkottmann
Copy link

We are currently upgrading our application to Federation v2. After upgrading the necessary packages, we found out that node-fetch is a runtime dependency of @apollo/gateway (current, v2.0.5) and is missing in its package.json.

We therefore get a runtime error:

Error: Cannot find module 'node-fetch'
Require stack:
- /var/task/node_modules/@apollo/gateway/dist/executeQueryPlan.js
- /var/task/node_modules/@apollo/gateway/dist/index.js 

Strange enough the only node-fetch related thing in the package.json are the typings of node-fetch @types/node-fetch which is sorted into the dependencies, although typing packages typically are listed in the devDependencies-section.

See https://github.com/apollographql/federation/blob/%40apollo/gateway%402.0.5/gateway-js/package.json

Adding node-fetch@2.6.7 as a dependency in our projects package.json fixed this problem for us, but is considered a workaround, because our codebase does not use node-fetch.

@benweatherman
Copy link
Contributor

howdy @marcelkottmann! Are you able to share the package file (at least the dependencies) and which package manager is being used? Can you also share more of the stacktrace, specifically the line the error is coming from?

although typing packages typically are listed in the devDependencies-section

I agree about this, it should be moved over to devDependencies

Doing some minimal setup seems to work OK https://stackblitz.com/edit/apollo-basic-fed-demo-twdpoe?file=package.json, though I realize that's probably not very helpful since it's not upgrading from a previous version.

@marcelkottmann
Copy link
Author

marcelkottmann commented Jul 7, 2022

Hi @benweatherman ,

I have no lines in the stack trace but I guess its

node_modules/@apollo/gateway/dist/executeQueryPlan.js:4 because there is this line

const node_fetch_1 = require("node-fetch");

So basically you are using node-fetch in your code, without declaring it as a production dependency.

We use npm and these are apollo-related top level project package.json in the dependencies section:

    "@apollo/gateway": "2.0.5",
    "apollo-server": "3.9.0",
    "apollo-server-cache-redis": "3.3.1",
    "apollo-server-caching": "3.3.0",
    "apollo-server-core": "3.9.0",
    "apollo-server-express": "3.9.0",
    "apollo-server-lambda": "3.9.0",
    "apollo-server-plugin-base": "3.6.1",
    "apollo-server-plugin-response-cache": "3.6.1",
    "apollo-server-types": "3.6.1",
    "graphql": "16.5.0",

Your test project you linked is not using npm (it is using turbo under the hood). If you would try that command on your project npm ci --only=production you might get the missing import node-fetch error.

@benweatherman benweatherman self-assigned this Jul 7, 2022
@benweatherman benweatherman added this to the 2.1.0 milestone Jul 7, 2022
@benweatherman
Copy link
Contributor

benweatherman commented Jul 7, 2022

Thanks for the extra info @marcelkottmann! Not sure how I missed these before but we import node-fetch in a few places now, e.g.

import { Headers } from 'node-fetch';

Not sure yet if should be importing those from somewhere else or if we should add the runtime dependency to node-fetch. Will do some investigation and circle back.

benweatherman added a commit that referenced this issue Jul 9, 2022
We do use `node-fetch` during runtime for some of the public methods in `RemoteGraphQLDataSource`. Using `node-fetch@2` because v3 is ESM-only. There's already a renovate rule to keep things from going to v3.

We should probably break the interfaces that are using `node-fetch`'s classes, since they aren't used by the default implementation after switching to `make-fetch-happen`.

### Other detritus

- **Move `@types/node-fetch` to `devDependencies`** This was originally included in apollographql/apollo-server#3546 as a fix for apollographql/apollo-server#3471. I think this is more appropriate for types.
- **Change some imports to use `type`** so there's no runtime dependency for things that are just using types.
- **Add `@types/make-fetch-happen`**
- **Remove `pretty-format`** since that's not a runtime thing

Fixes #1961
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants