Skip to content
This repository has been archived by the owner on Aug 15, 2024. It is now read-only.

Docs: Update copy regarding poly/ponyfills #158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Docs: Update copy regarding poly/ponyfills #158

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 11 additions & 15 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
Official Javascript wrapper for the [Unsplash API](https://unsplash.com/developers).

Key Links:

- Before using the Unsplash API, [register as a developer](https://unsplash.com/developers).
- Before using the Unsplash API, read the [API Guidelines](https://help.unsplash.com/api-guidelines/unsplash-api-guidelines). Specifically, you _must_:
- [hotlink images](https://help.unsplash.com/api-guidelines/more-on-each-guideline/guideline-hotlinking-images)
Expand Down Expand Up @@ -36,38 +37,33 @@ $ yarn add unsplash-js

### Fetch

This library depends on [fetch](https://fetch.spec.whatwg.org/) to make requests to the Unsplash API. For environments that don't support fetch, you'll need to provide polyfills of your choosing. Here are the ones we recommend:

- node implementation: [node-fetch](https://github.com/bitinn/node-fetch)
- browser polyfill: [whatwg-fetch](https://github.com/github/fetch)
This library depends on [fetch](https://fetch.spec.whatwg.org/) to make requests to the Unsplash API. For environments that don't support fetch, you'll need to provide polyfills of your choosing. We recommend using [isomorphic-unfetch](https://github.com/developit/unfetch/tree/master/packages/isomorphic-unfetch).

#### Adding polyfills

`createApi` receives an optional `fetch` parameter. When it is not provided, we rely on the globally scoped `fetch`.

This means that you can set the polyfills in the global scope:
This means that you can set the polyfill in the global scope:

```ts
// server
import fetch from 'node-fetch';
global.fetch = fetch;
import 'isomorphic-unfetch';

// browser
import 'whatwg-fetch';
// in a browser env, this will set window.fetch = fetch;
// in a node env, this will set global.fetch = fetch;
```

or explicitly provide them as an argument:
or as a [ponyfill](https://github.com/sindresorhus/ponyfill) by explicitly providing it as an argument:

```ts
import nodeFetch from 'node-fetch';
import fetch from 'isomorphic-unfetch';

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

Note: we recommend using a version of `node-fetch` higher than `2.4.0` to benefit from Brotli compression.
Note: If you choose to use [node-fetch](https://github.com/node-fetch/node-fetch), we recommend using a version higher than `2.4.0` to benefit from Brotli compression. In addition, you may need to import an `AbortController` to patch TypeScript-related issues.
Copy link
Member

Choose a reason for hiding this comment

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

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).

Copy link
Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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).

Copy link
Author

Choose a reason for hiding this comment

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

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.


### URL

Expand All @@ -76,7 +72,7 @@ This library also depends on the WHATWG URL interface:
- MDN [docs](https://developer.mozilla.org/en-US/docs/Web/API/URL) for browsers.
- NodeJS [docs](https://nodejs.org/api/url.html).

Note: Make sure to polyfill this interface if targetting older environments that do not implement it (i.e. Internet Explorer or Node < v8).
Note: Make sure to polyfill this interface if targeting older environments that do not implement it (i.e. Internet Explorer or Node < v8).

Note 2: For Node, the URL interface exists under `require('url').URL` since [v8](https://nodejs.org/es/blog/release/v8.0.0/#say-hello-to-the-whatwg-url-parser) but was only added to the global scope as of [v10.0.0](https://nodejs.org/docs/latest/api/globals.html#globals_url). If you are using a version between v8.0.0 and v10.0.0, you need to add the class to the global scope before using `unsplash-js`:

Expand Down