Skip to content
This repository has been archived by the owner on Apr 4, 2022. It is now read-only.

Add TypeScript types to fetchFunc option #1098

Merged
merged 1 commit into from
Aug 3, 2021

Conversation

dstaley
Copy link
Member

@dstaley dstaley commented Jul 30, 2021

This PR adds more specific TypeScript types to the fetchFunc option.

Using Function is the same as (...args: any) => any, meaning that any function would be accepted as a valid fetchFunc. With this PR, the supplied function will need to conform to the minimal fetch interface. Essentially, functions will need to have the following properties:

export interface FetchHeaders {
  keys(): IterableIterator<string> | string[];
  entries(): IterableIterator<[string, string]> | [string, string][];
  get(name: string): string | null;
  has(name: string): boolean;
}

export interface FetchResponse {
  status: number;
  statusText: string;
  text(): Promise<string>;
  headers: FetchHeaders;
}

export type FetchFunction = (
  input: RequestInfo,
  init?: RequestInit | undefined
) => Promise<FetchResponse>;

Most of this should be easily achievable with XMLHttpRequest, and even easier if you can reuse the browser's Headers class.

I'm fairly confident this also covers all our usage in kinto.js, but if not I'll update the types here to ensure that responses from kinto-http.js have all the bits that kinto.js wants.

(Also, since the new fetchFunc option is for Firefox, these types won't be enforced. They will, however, be a good guide to follow when building since they represent all the functionality that kinto-http.js expects from fetch.)

@dstaley dstaley requested a review from leplatrem July 30, 2021 19:02
@github-actions
Copy link

Size Change: -6 B (0%)

Total Size: 40.9 kB

ℹ️ View Unchanged
Filename Size Change
dist/kinto-http.min.js 9.09 kB 0 B
dist/kinto-http.node.js 15.2 kB 0 B
dist/moz-kinto-http-client.js 16.6 kB -6 B (0%)

compressed-size-action

Copy link
Contributor

@leplatrem leplatrem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing my «haste» 😊

I have a question (that may have an obvious answer but I don't see it): why do we redefine Response and Headers instead of using official types (like you did for RequestInfo and RequestInit)?

@dstaley
Copy link
Member Author

dstaley commented Aug 2, 2021

I have a question (that may have an obvious answer but I don't see it): why do we redefine Response and Headers instead of using official types (like you did for RequestInfo and RequestInit)?

Great question! The reason is that Response and Headers have very large interfaces that might be a pain to implement. For instance, here's the interfaces for Response and Headers:

interface Response {
    headers: Headers;
    ok: boolean;
    redirected: boolean;
    status: number;
    statusText: string;
    type: ResponseType;
    url: string;
    clone(): Response;
    body: ReadableStream<Uint8Array> | null;
    bodyUsed: boolean;
    arrayBuffer(): Promise<ArrayBuffer>;
    blob(): Promise<Blob>;
    formData(): Promise<FormData>;
    json(): Promise<any>;
    text(): Promise<string>;
}

interface Headers {
    append(name: string, value: string): void;
    delete(name: string): void;
    get(name: string): string | null;
    has(name: string): boolean;
    set(name: string, value: string): void;
    forEach(callbackfn: (value: string, key: string, parent: Headers) => void, thisArg?: any): void;
}

In order for TypeScript to compile a user's code, the user would need to implement the entirety of those interfaces. By using our own subset of the interface, we can limit the interface to just the behavior that we rely on.

You might be wondering why we use RequestInit instead of defining our own interface. Well, since RequestInit is an input instead of an output, you don't need to do any additional work to handle it. All the implementation effort is around making the return value coform to the interfaces, so we can save us some work by reusing the input interfaces.

@leplatrem
Copy link
Contributor

Merci for the detailed explanation 🙏! That makes total sense :)

@leplatrem leplatrem merged commit 4caa989 into introduce-fetch-hook Aug 3, 2021
@leplatrem leplatrem deleted the fetchfunc-types branch August 3, 2021 08:22
leplatrem added a commit that referenced this pull request Aug 4, 2021
* Add  option to client constructor

* Fix lint

* Define globalThis for Firefox build

Without this the exported symbol `KintoHttpClient` is not reachable.
This build has change that is responsible of this regression:

```diff

 (function (global, factory) {
     typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() :
     typeof define === 'function' && define.amd ? define(factory) :
-    (global = global || self, global.KintoHttpClient = factory());
+    (global = typeof globalThis !== 'undefined' ? globalThis : global || self, global.KintoHttpClient = factory());
 }(this, (function () { 'use strict';

```

* add types to fetchFunc option (#1098)

Co-authored-by: Dylan Staley <88163+dstaley@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants