Docs: Update copy regarding poly/ponyfills

[msutkowski]

Overview

Updates copy around node usage and prefers isomorphic-unfetch, which wraps both of the currently suggested libs. Additionally, this functions as a polyfill, while node-fetch itself will only work as a ponyfill.

If someone were to try to use node-fetch directly as recommended in the docs in a TS project, they're more likely to run into a few issues. Depending on the node-fetch version used, a user may need to import abort-controller and set it on global. Additionally, any version > 3 for a TS user would currently see an error like when making a request:

message: 'Expected signal to be an instanceof AbortSignal',

[OliverJAsh]

TIL about unfetch/isomorphic-unfetch! Thanks for contributing.

[OliverJAsh]

I think this note about Brotli compression also applies to isomorphic-unfetch—not just node-fetch. We should update this comment to reflect that, i.e. mention the minimum version of isomorphic-unfetch that supports Brotli compression (uses node-fetch higher than 2.4.0).

[msutkowski]

Good point - isomorphic-unfetch should be fine assuming a user is on the latest)

[OliverJAsh]

In addition, you may need to import an AbortController to patch TypeScript-related issues.

Is this referring to the other comment you left in the PR description (below)?

Additionally, any version > 3 for a TS user would currently see an error like when making a request:

message: 'Expected signal to be an instanceof AbortSignal',

If so, I'm not sure I understand what the problem is. It looks like the error message you mentioned is a runtime error, thrown by node-fetch, not a TS (compile time) type error?

Perhaps you could provide a reduced test case of the problem to help me understand?

I tried copying the example from our docs:

import 'isomorphic-unfetch';

import { createApi } from 'unsplash-js';

const unsplash = createApi({
    accessKey: 'MY_ACCESS_KEY',
});

const controller = new AbortController();
const signal = controller.signal;

unsplash.photos.get({ photoId: '123' }, { signal }).catch((err) => {
    if (err.name === 'AbortError') {
        console.log('Fetch aborted');
    }
});

controller.abort();

Using TS 4 I have no type errors. When I try to run this in Node I do get a runtime error, but it is different from the one you mentioned:

const controller = new AbortController();
                   ^

ReferenceError: AbortController is not defined

For that, I think we should add a note to the docs that an AbortController polyfill/ponyfill is required in Node (separate to the changes in this PR).

[msutkowski]

Sorry about that - I shouldn't work at night. You're correct, it's a run-time error, not a TS error.

The confusing part to this is the current behaviors between node-fetch 2.6 and 3, and I was trying to account for the edge cases. (I tested isomorphic-unfetch, cross-fetch, node-fetch with and without abort controllers)

Thanks again for your review, I'll go ahead and do a cleanup pass on this shortly.

[msutkowski]

@OliverJAsh After a little more thought, I'd also like to document getting things working for TS users in a node env as well. As an example, this kind of thing is necessary:

// ./src/global.d.ts
import _fetch, { RequestInit as _RequestInit } from 'node-fetch';

declare global {
  export interface RequestInit extends _RequestInit {
    cache?: any;
    credentials?: any;
    integrity?: any;
    keepalive?: any;
    mode?: any;
    referrer?: any;
    referrerPolicy?: any;
    window?: any;
  }
  export const fetch: typeof _fetch;
}

You can't do this with cross-fetch or isomorphic-unfetch due to how they export types, but it is possible with node-fetch. Ideally, these libraries would also polyfill the global types as well as the run time, but they do not. In this case, a user could use isomorphic-unfetch and npm i @types/node-fetch and do what's shown above, but it might make sense just to revert back to the original recommendations.

Which way makes the most sense to you?

Edit: as another note, someone could just add 'dom' to lib in their tsconfig, but I'd advise against recommending this as its not accurate and would be a potentially 'broken' experience.