Types of property 'headers' are incompatible (with new 3.1.0)

[Jauminha]

(First of all thanks for this amazing package!)

We are using cross-fetch to use the same client calls to our APIs from other APIs and any running application. For our infrastructural needs, we created a small customFetch function on top of the fetch function so we could change the URL if a cloud parameter is passed or not:

export const customFetch = (input: RequestInfo, init: RequestInit, cloud?: CloudSubdomain | string): Promise<Response> => {
    let url = "";
    if (cloud) {
        url += createCloudUrl(cloud);
    }
    url += input;
    return fetch(url, init);
};

After updating to version 3.1.0 our code started to throw the following error:

Type 'Promise<import("/home/user/development/node_modules/cross-fetch/lib.fetch").Response>' is not assignable to type 'Promise<Response>'.
  Type 'import("/home/user/development/node_modules/cross-fetch/lib.fetch").Response' is not assignable to type 'Response'.
    Types of property 'headers' are incompatible.
      Type 'Headers' is missing the following properties from type 'Headers': [Symbol.iterator], entries, keys, values

22     return fetch(url, init);

I inspected the types from both node_modules/typescript/lib/lib.dom.d.ts and node_modules/cross-fetch/lib.fetch.d.ts and I see no difference apart from this:

declare var Headers: {
    prototype: Headers;
    new(init?: HeadersInit): Headers;
};

Hope you can shed some light on this. Thanks for your time!

[jstewmon]

The problem is that we're missing an augmentation from lib.dom.iterable.d.ts:

interface Headers {
    [Symbol.iterator](): IterableIterator<[string, string]>;
    /**
     * Returns an iterator allowing to go through all key/value pairs contained in this object.
     */
    entries(): IterableIterator<[string, string]>;
    /**
     * Returns an iterator allowing to go through all keys of the key/value pairs contained in this object.
     */
    keys(): IterableIterator<string>;
    /**
     * Returns an iterator allowing to go through all values of the key/value pairs contained in this object.
     */
    values(): IterableIterator<string>;
}

I think I can fix this by adding a graft for that specific interface and use declaration merging to get the right type definition in the cross-fetch index.d.ts.

[Jauminha]

Awesome! Waiting for the merge 👍

[lquixada]

Fix released on 3.1.2. 👍 thanks everyone!

[ThomWright]

@lquixada This is still happening for me on 3.1.2.

[jstewmon]

🤦 Sorry again - I was trying to quickly think of a way to fix this problem and I failed to really internalize the problem before proposing a solution.

I've got a new approach that I think will really work. I'll open a new PR shortly, including how I've tested it against the code sample in this issue.

[Jauminha]

I guess the issue should be reopened until the fix has been merged? More people will stumble upon this

[lquixada]

@Jauminha yeah, makes sense. I'm still working on this though!

[lquixada]

@Jauminha just released an alpha version. can you try your code with npm install cross-fetch@3.1.3-alpha.4

[iCrawl]

This does not work at all now, because fetch is now a type only and cannot be used as a value/function anymore.

[lquixada]

@iCrawl hi, can you try again with npm install cross-fetch@3.1.3-alpha.6?

[Jauminha]

Sorry, email notifications went into spam. Alpha.4 same as iCrawl. Alpha.6 👍, works for me.

[jstewmon]

@lquixada was there a problem with the index.d.ts from #100 you were trying to fix?

The changes you made put us nearly full circle where importing cross-fetch pollutes the globals with all the types that are in lib.fetch.d.ts. The folks using dom lib don't notice any problem b/c they already have all those types in their globals.

But, server app developers get incorrect type checking feedback. For example, this should not compile without using the dom lib, but it does because the compiler finds a global type definition for Request:

import fetch from "cross-fetch";

const req = new Request("http://example.com");

[lquixada]

@jstewmon I feel the issue we're trying to address here is the conflict reported by @Jauminha regarding Headers (and any other fetch API member). So far cross-fetch@3.1.3-alpha.6 seems to fix the problem.

Regarding the code you've mentioned, not sure why it should not compile. In Typescript, global types are a valid way to provide typing.

import fetch from "cross-fetch";

const req = new Request("http://example.com");

The new cross-fetch version provides types from both imports and globals. The reason for this is that it can be used as a ponyfill (import fetch from "cross-fetch") or as a polyfill (import "cross-fetch/poyfill";). PR #100 doesn't address the latter case.

That being said, I'm not saying that the current approach (3.1.3-alpha.6) should be the final one. We could certainly go back to discuss it but probably in another github issue or PR.

[jstewmon]

Regarding the code you've mentioned, not sure why it should not compile. In Typescript, global types are a valid way to provide typing.

In the example, I used Request as a class (concrete type), not an interface (abstract type), so the code will fail at runtime on the server. To be correct, it must be written as:

import fetch, { Request } from "cross-fetch";

const req = new Request("https://example.com");

The reason for this is that it can be used as a ponyfill (import fetch from "cross-fetch") or as a polyfill (import "cross-fetch/poyfill";). PR #100 doesn't address the latter case.

This requires separate type definition files. The global vars and types should be declared in polyfill.d.ts, which only needs the concrete declarations which are globally polyfilled and their supporting type declarations (no imports / exports are needed).

[lquixada]

@jstewmon interesting approach! Do you feel we should open a new issue for that?

[jstewmon]

I would say this has been a rather tricky issue to sort out, and this discussion has revealed much of the nuance in addressing it properly, so I lean towards keeping the details consolidated here. But, I don't think the choice has much consequence either way, so if you prefer a new issue, that's fine too.

[lquixada]

thanks all! 3.1.3 has just been released 👍

@jstewmon yeah I agree it's a tricky issue. I've released a new version so everyone gets the compilation error solved. Let's open a new issue/PR to address the global pollution and the polyfill case! Closing this one for now.