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.

Sign up for GitHub

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

Jump to bottom

@apollo/gateway doesn't include node-fetch in its dependencies #3471

Closed
migueloller opened this issue Nov 4, 2019 · 1 comment · Fixed by #3546
Closed

@apollo/gateway doesn't include node-fetch in its dependencies #3471

migueloller opened this issue Nov 4, 2019 · 1 comment · Fixed by #3546
Milestone
Release 2.9.13

Comments

@migueloller
Copy link

When using @apollo/gateway@0.10.8, TypeScript fails because it can't find the types for node-fetch. This happens specifically here. It looks like node-fetch should be added as a dependency, or at least @types/node-fetch.
@msheakoski
Copy link

I ran into this recently and had to add @types/node-fetch to my devDependencies in order to fix failing builds. Depending on the order that you add/update dependencies, node-fetch usually manages to sneak in, but if you rm -rf node_modules yarn.lock && yarn install, the missing node-fetch error happens pretty consistently.

I don't use node-fetch directly in my project, and since you can't have comments in package.json, the dependency will end up becoming one of those random things that everyone needs to remember in order to not break builds.

I really hope this gets fixed soon for better DX and reduced element of surprise.

abernix added a commit that referenced this issue Nov 22, 2019
@abernix
Add production dependency on @types/node-fetch.
8a583bd 
As the `HeadersInit` is exported and used in the exported [`GatewayConfig`]
type via [`RemoteGatewayConfig`] - which is emitted to `@apollo/gateway`'s
`dist/index.d.ts` declaration, the `node-fetch` types need to be available
by those who install the package.

[`GatewayConfig`]: https://github.com/apollographql/apollo-server/blob/73cdf0d533908894282debdb6dc5444186af27a6/packages/apollo-gateway/src/index.ts#L74-L77
[`RemoteGatewayConfig`]: https://github.com/apollographql/apollo-server/blob/73cdf0d533908894282debdb6dc5444186af27a6/packages/apollo-gateway/src/index.ts#L62

Fixes: #3471
@abernix abernix mentioned this issue Nov 22, 2019
Merged
abernix added a commit that referenced this issue Nov 27, 2019
@abernix
Add production dependency on @types/node-fetch. (#3546)
c63786b 
* Add production dependency on `@types/node-fetch`.

As the `HeadersInit` is exported and used in the exported [`GatewayConfig`]
type via [`RemoteGatewayConfig`] - which is emitted to `@apollo/gateway`'s
`dist/index.d.ts` declaration, the `node-fetch` types need to be available
by those who install the package.

[`GatewayConfig`]: https://github.com/apollographql/apollo-server/blob/73cdf0d533908894282debdb6dc5444186af27a6/packages/apollo-gateway/src/index.ts#L74-L77
[`RemoteGatewayConfig`]: https://github.com/apollographql/apollo-server/blob/73cdf0d533908894282debdb6dc5444186af27a6/packages/apollo-gateway/src/index.ts#L62

Fixes: #3471

* Add CHANGELOG.md for #3546.
@ghost ghost mentioned this issue Dec 4, 2019
Closed
@abernix abernix added this to the Release 2.9.13 milestone Dec 17, 2019
abernix added a commit to apollographql/federation that referenced this issue Sep 4, 2020
@abernix
Add production dependency on @types/node-fetch. (apollographql/apol…
3f16c7a 
…lo-server#3546)

* Add production dependency on `@types/node-fetch`.

As the `HeadersInit` is exported and used in the exported [`GatewayConfig`]
type via [`RemoteGatewayConfig`] - which is emitted to `@apollo/gateway`'s
`dist/index.d.ts` declaration, the `node-fetch` types need to be available
by those who install the package.

[`GatewayConfig`]: https://github.com/apollographql/apollo-server/blob/73cdf0d533908894282debdb6dc5444186af27a6/packages/apollo-gateway/src/index.ts#L74-L77
[`RemoteGatewayConfig`]: https://github.com/apollographql/apollo-server/blob/73cdf0d533908894282debdb6dc5444186af27a6/packages/apollo-gateway/src/index.ts#L62

Fixes: apollographql/apollo-server#3471

* Add CHANGELOG.md for apollographql/apollo-server#3546.

Apollo-Orig-Commit-AS: apollographql/apollo-server@c63786b
benweatherman added a commit to apollographql/federation that referenced this issue Jul 9, 2022
@benweatherman
Add runtime dependency on node-fetch
ed4bd5d 
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
@benweatherman benweatherman mentioned this issue Jul 9, 2022
Merged
2 tasks
benweatherman added a commit to apollographql/federation that referenced this issue Jul 18, 2022
@benweatherman
Add runtime dependency on node-fetch (#1970)
0fd83f0 
We 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

- **Remove `@types/node-fetch`** This was originally included in apollographql/apollo-server#3546 as a fix for apollographql/apollo-server#3471.
- **Change some imports to use `type`** so there's no runtime dependency for things that are just using types.
- **Remove `pretty-format`** since that's not a runtime thing
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release 2.9.13
Development

Successfully merging a pull request may close this issue.

Add production dependency on @types/node-fetch. apollographql/apollo-server
3 participants
@msheakoski @abernix @migueloller