prismicio/prismic-client

Typescript/Promise signal type issue

Closed this issue · 4 comments

sarunast commented

Versions

  • @prismicio/client: from 6.6.2
  • typescript: 4.7.4

Reproduction

With typescript 4.7.4 and the recent signal changes from this PR, #250 it breaks the types. With the older versions of typescript it works fine.

Typescript error:

Argument of type 'RequestInitLike | undefined' is not assignable to parameter of type 'RequestInit | undefined'.
  Type 'RequestInitLike' is not assignable to type 'RequestInit'.
    Types of property 'signal' are incompatible.
      Type 'AbortSignalLike | null | undefined' is not assignable to type 'AbortSignal | null | undefined'.
        Type 'AbortSignalLike' is missing the following properties from type 'AbortSignal': reason, throwIfAborted(2345)

The error can be inspected on this playground.
https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBDAnmApnA3nAYimBjACwBlgBrNAXzgDMoIQ4AiAATCmAGcRg9gIB6PABtgKAHYxGAbgBQoSLDgBFAK49SRAEoBVGnQaMAjmrykAtEKgrpMmXghiO8PAENCaALxwxKAO7KTDR0AHid2MQBzABo4TRQOSEcUAD4ACnQZODgQFwAPAGVgAC8UAC44AEYABhqY-n5Kmqq4cRh2eKjM7LyAQQiyxpq4ACo4ADZm0YBWOoap7OAxFRh4mQoASlk7Byc4V3cAExx8AnLjwhJyOC8XDkQxPDhUlSghGIgwGD5HdevkjC69TgABUCGh9mC4OREHsHDAXIsOAhIVAUMZ4isDnBtJoiHAXGIsWCXAcUFAOF17I54NDrnAAFL5ADyADkAHRhRYRYDURDpOAvN5wD5fHZwDayLJU3YQlAHOIJHaePZuMFs-owVLQza2LI8p5IVAQagqw4KxIcNAAQg8XkYKkJKGoizljF+GSyWVRMBeYlNYPl8QtKDZwgcKFSOqyVBQQktAM9sOpcFRSJuvgR8GouEIz1e70+3w4Oq6epNqVTbIgpHdpc9so5uC1KEQMUrYZ8kajnoous93t9KfikvFay2KFyCngpOoLhUQmcqrl5wIsiAA

lihbr commented

Hey there, thank you so much for the comprehensive report!

Looks like the AbortSignal type is quite all over the place right now:

I guess we should revert #250 to match the "official" TypeScript DOM interface, or maybe we could just rely on the RequestInit type (ky does so for example: https://github.com/sindresorhus/ky/blob/main/source/types/options.ts#L205-L223) but I'm not sure anymore why we had to go with RequestInitLike 🤔

wdyt @angeloashmore?

sarunast commented

The actual types for the TS 4.7.4 (the breaking changes). On the main branch the types somehow are not reflected yet 🤔

The new breaking types:
https://github.com/microsoft/TypeScript/blob/v4.7.4/lib/lib.dom.d.ts#L1980-L1990

Basically, these props were added in v4.7 to AbortSignal. That's why it fails.

    readonly reason: any;
    throwIfAborted(): void;
  • 👍1
angeloashmore commented

Hey @sarunast, thanks for reporting this.

As @lihbr shared, we're dealing with multiple incompatible type definitions for AbortSignal. Some require properties that don't exist in others, like reason and throwIfAborted().

We may need to fall back to using { signal: any } to support the incompatible types, but we will need to do some exploration before making that change. Ideally, we could find some cross-compatible way of typing AbortSignal, but it's not obvious how that could be accomplished (to me, at least).

If this is blocking you, I recommend using // @ts-expect-error to suppress the error for now. You can also use a previous version of @prismicio/client, but I don't recommend that.

angeloashmore commented

I guess we should revert #250 to match the "official" TypeScript DOM interface, or maybe we could just rely on the RequestInit type (ky does so for example: https://github.com/sindresorhus/ky/blob/main/source/types/options.ts#L205-L223) but I'm not sure anymore why we had to go with RequestInitLike 🤔

RequestInitLike was introduced to avoid type incompatibility issues, similar to what we're experiencing with AbortSignal.

Different fetch implementations use their own types, potentially with subtle differences. node-fetch@3's fetch(), for example, is incompatible with ky's fetch() type:

import fetch from "node-fetch";
import ky from 'ky'

await ky("https://qwerty.cdn.prismic.io/api/v2", { fetch })

TypeScript Playground: https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAMwKYwMYAtFQiOAiAOwgBMkBaZNdPAbgChRJY4BrATyxzgHJ3u66AQwDug4PHYAKPOhgwwAZwBcAehUBHYUlhsAdKmIFdYKMAUhgqXcAgrBYYCoBuAJjwAaOAG9EKDHABfAEogA

While we technically should use the "most correct" approach and use Request, RequestInit, Response, etc. directly, it leads to issues in real-world projects that use libraries like node-fetch.

To get around this, we can create our own minimal types that form a general definition only including what we need. This is where RequestInitLike comes in: it only includes properties we need and tries to be generic so as to support different RequestInit implementations.

I suspect Ky can use the global types directly because Ky is a browser library. It does not expect nor support developers to pass, for example, node-fetch as a fetch() implementation.

Hope that makes sense. It may be worth exploring the removal of this strategy, but generally speaking, it has worked well, save for AbortSignal.

  • 👍1