jasonkuhrt / graphql-request

Minimal GraphQL client
MIT License
5.74k stars 307 forks source link

Is `cross-fetch` dependency needed? Can you remove it? #940

Closed UchihaYuki closed 1 week ago

UchihaYuki commented 1 week ago

Screenshot

node_modules/@uchihayuki/js-graphql-client/node_modules/cross-fetch/index.d.ts:3:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: _fetch, _Request, _Response, _Headers, fetch, Request, Response, Headers

3 declare const _fetch: typeof fetch;
  ~~~~~~~

  node_modules/cross-fetch/index.d.ts:3:1
    3 declare const _fetch: typeof fetch;
      ~~~~~~~
    Conflicts are in this file.
>npm list cross-fetch  
express-common@ C:\Data\projects\npm-packages\projects\express-common
├─┬ u/graphql-codegen/cli@5.0.2
│ └─┬ @graphql-tools/prisma-loader@8.0.4
│   └─┬ graphql-request@6.1.0
│     └── cross-fetch@3.1.8
├─┬ @graphql-codegen/typed-document-node@5.0.7
│ └─┬ @graphql-codegen/visitor-plugin-common@5.2.0
│   └─┬ @graphql-tools/relay-operation-optimizer@7.0.1
│     └─┬ @ardatan/relay-compiler@12.0.0
│       └─┬ fbjs@3.0.5
│         └── cross-fetch@3.1.8 deduped
└─┬ @uchihayuki/js-graphql-client@0.1.0
  └── cross-fetch@4.0.0

Description

@uchihayuki/js-graphql-client is written by me, it's a private package. I just found this problem yesterday. I think both of us should remove cross-fetch as a dependency and let the package consumer choose their version, or conflicts will happen. They can include a polyfill.ts in their main index.ts like the following:

import fetch from "cross-fetch";
globalThis.fetch = fetch;

or they can pass the implementation of fetch to your client

import { gql, GraphQLClient } from 'graphql-request';
import fetch from "cross-fetch";

const document = gql`
  {
    company {
      ceo
    }
  }
`
const endpoint = 'https://api.spacex.land/graphql/'
const client = new GraphQLClient(endpoint, fetch)
await client.request(document)

My point is we shouldn't list that library as dependencies or it will pollute global space.

I haven't read through your code, but does something like the following work for you?

type Fetch = typeof fetch; 

Reproduction Steps/Repo Link

jonkoops commented 1 week ago

This dependency has already been removed https://github.com/jasonkuhrt/graphql-request/pull/597