Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api-fetch: Type the rest of the package #30161

Merged
merged 5 commits into from
Mar 25, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 2 additions & 0 deletions packages/api-fetch/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- Publish TypeScript definitions.

## 3.22.0 (2021-03-17)

## 3.8.1 (2019-04-22)
Expand Down
63 changes: 51 additions & 12 deletions packages/api-fetch/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
* Default set of header values which should be sent with every request unless
* explicitly provided through apiFetch options.
*
* @type {Object}
* @type {Record<string, string>}
*/
const DEFAULT_HEADERS = {
// The backend uses the Accept header as a condition for considering an
Expand All @@ -43,17 +43,32 @@ const DEFAULT_OPTIONS = {
credentials: 'include',
};

/**
* @type {import('./types').ApiFetchMiddleware[]}
*/
const middlewares = [
userLocaleMiddleware,
namespaceEndpointMiddleware,
httpV1Middleware,
fetchAllMiddleware,
];

/**
* Register a middleware
*
* @param {import('./types').ApiFetchMiddleware} middleware
*/
function registerMiddleware( middleware ) {
middlewares.unshift( middleware );
}

/**
* Checks the status of a response, throwing the Response as an error if
* it is outside the 200 range.
*
* @param {Response} response
* @return {Response} The response if the status is OK.
sarayourfriend marked this conversation as resolved.
Show resolved Hide resolved
*/
const checkStatus = ( response ) => {
if ( response.status >= 200 && response.status < 300 ) {
return response;
Expand All @@ -62,6 +77,11 @@ const checkStatus = ( response ) => {
throw response;
};

/** @typedef {(options: import('./types').ApiFetchRequestProps) => Promise<any>} FetchHandler*/

/**
* @type {FetchHandler}
*/
const defaultFetchHandler = ( nextOptions ) => {
const { url, path, data, parse = true, ...remainingOptions } = nextOptions;
let { body, headers } = nextOptions;
Expand All @@ -75,7 +95,13 @@ const defaultFetchHandler = ( nextOptions ) => {
headers[ 'Content-Type' ] = 'application/json';
}

const responsePromise = window.fetch( url || path, {
const resolvedFetchUrl = url || path;

if ( typeof resolvedFetchUrl === 'undefined' ) {
throw new Error( 'Please specify either a `path` or `url`' );
}

const responsePromise = window.fetch( resolvedFetchUrl, {
sarayourfriend marked this conversation as resolved.
Show resolved Hide resolved
...DEFAULT_OPTIONS,
...remainingOptions,
body,
Expand Down Expand Up @@ -107,25 +133,34 @@ const defaultFetchHandler = ( nextOptions ) => {
);
};

/** @type {FetchHandler} */
let fetchHandler = defaultFetchHandler;

/**
* Defines a custom fetch handler for making the requests that will override
* the default one using window.fetch
*
* @param {Function} newFetchHandler The new fetch handler
* @param {FetchHandler} newFetchHandler The new fetch handler
*/
function setFetchHandler( newFetchHandler ) {
fetchHandler = newFetchHandler;
}

/**
* @template T
* @param {import('./types').ApiFetchRequestProps} options
* @return {Promise<T>} A promise representing the request processed via the registered middlewares.
*/
function apiFetch( options ) {
// creates a nested function chain that calls all middlewares and finally the `fetchHandler`,
// converting `middlewares = [ m1, m2, m3 ]` into:
// ```
// opts1 => m1( opts1, opts2 => m2( opts2, opts3 => m3( opts3, fetchHandler ) ) );
// ```
const enhancedHandler = middlewares.reduceRight( ( next, middleware ) => {
const enhancedHandler = middlewares.reduceRight( (
/** @type {FetchHandler} */ next,
middleware
) => {
return ( workingOptions ) => middleware( workingOptions, next );
}, fetchHandler );

Expand All @@ -135,14 +170,18 @@ function apiFetch( options ) {
}

// If the nonce is invalid, refresh it and try again.
return window
.fetch( apiFetch.nonceEndpoint )
.then( checkStatus )
.then( ( data ) => data.text() )
.then( ( text ) => {
apiFetch.nonceMiddleware.nonce = text;
return apiFetch( options );
} );
return (
window
// @ts-ignore
Copy link
Member

Choose a reason for hiding this comment

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

Were we going to type this one, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I wasn't sure where these came from or how to type them effectively without having to declare the types for all the things that TS is doing a good job of inferring. How does the nonceMiddleware end up on apiFetch?

Copy link
Member

Choose a reason for hiding this comment

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

I see. Well, this is a mess 😉 Should it have been a part of the nonce middleware instead? It's definitely something we can fix in another PR anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Just saw this. nonceMiddleware and nonceEndpoint are being used in Core:

https://github.com/WordPress/wordpress-develop/blob/fc6ae7b5f68770c6864b8d725f0c80824fc7a27e/src/wp-includes/script-loader.php#L310-L319

It definitely can use some improvements though 😅 .

.fetch( apiFetch.nonceEndpoint )
.then( checkStatus )
.then( ( data ) => data.text() )
.then( ( text ) => {
// @ts-ignore
apiFetch.nonceMiddleware.nonce = text;
return apiFetch( options );
} )
);
} );
}

Expand Down
43 changes: 35 additions & 8 deletions packages/api-fetch/src/middlewares/fetch-all-middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,32 @@ import { addQueryArgs } from '@wordpress/url';
*/
import apiFetch from '..';

// Apply query arguments to both URL and Path, whichever is present.
/**
* Apply query arguments to both URL and Path, whichever is present.
*
* @param {import('../types').ApiFetchRequestProps} props
* @param {Record<string, string | number>} queryArgs
* @return {import('../types').ApiFetchRequestProps} The request with the modified query args
*/
const modifyQuery = ( { path, url, ...options }, queryArgs ) => ( {
...options,
url: url && addQueryArgs( url, queryArgs ),
path: path && addQueryArgs( path, queryArgs ),
} );

// Duplicates parsing functionality from apiFetch.
/**
* Duplicates parsing functionality from apiFetch.
*
* @param {Response} response
* @return {Promise<any>} Parsed response json.
*/
const parseResponse = ( response ) =>
response.json ? response.json() : Promise.reject( response );

/**
* @param {string | null} linkHeader
* @return {{ next?: string }} The parsed link header.
*/
const parseLinkHeader = ( linkHeader ) => {
if ( ! linkHeader ) {
return {};
Expand All @@ -31,22 +46,34 @@ const parseLinkHeader = ( linkHeader ) => {
: {};
};

/**
* @param {Response} response
* @return {string | undefined} The next page Url.
sarayourfriend marked this conversation as resolved.
Show resolved Hide resolved
*/
const getNextPageUrl = ( response ) => {
const { next } = parseLinkHeader( response.headers.get( 'link' ) );
return next;
};

/**
* @param {import('../types').ApiFetchRequestProps} options
* @return {boolean} True if the request contains an unbounded query.
*/
const requestContainsUnboundedQuery = ( options ) => {
const pathIsUnbounded =
options.path && options.path.indexOf( 'per_page=-1' ) !== -1;
!! options.path && options.path.indexOf( 'per_page=-1' ) !== -1;
const urlIsUnbounded =
options.url && options.url.indexOf( 'per_page=-1' ) !== -1;
!! options.url && options.url.indexOf( 'per_page=-1' ) !== -1;
return pathIsUnbounded || urlIsUnbounded;
};

// The REST API enforces an upper limit on the per_page option. To handle large
// collections, apiFetch consumers can pass `per_page=-1`; this middleware will
// then recursively assemble a full response array from all available pages.
/**
* The REST API enforces an upper limit on the per_page option. To handle large
* collections, apiFetch consumers can pass `per_page=-1`; this middleware will
* then recursively assemble a full response array from all available pages.
*
* @type {import('../types').ApiFetchMiddleware}
*/
const fetchAllMiddleware = async ( options, next ) => {
if ( options.parse === false ) {
// If a consumer has opted out of parsing, do not apply middleware.
Expand Down Expand Up @@ -81,7 +108,7 @@ const fetchAllMiddleware = async ( options, next ) => {
}

// Iteratively fetch all remaining pages until no "next" header is found.
let mergedResults = [].concat( results );
let mergedResults = [ ...results ];
sarayourfriend marked this conversation as resolved.
Show resolved Hide resolved
while ( nextPage ) {
const nextResponse = await apiFetch( {
...options,
Expand Down
2 changes: 1 addition & 1 deletion packages/api-fetch/src/middlewares/nonce.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* @param {string} nonce
* @return {import('../types').ApiFetchMiddleware} A middleware to enhance a request with a nonce.
* @return {import('../types').ApiFetchMiddleware & { nonce: string }} A middleware to enhance a request with a nonce.
*/
function createNonceMiddleware( nonce ) {
/**
Expand Down
14 changes: 5 additions & 9 deletions packages/api-fetch/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,13 @@
"declarationDir": "build-types"
},
"references": [
{ "path": "../i18n" },
Copy link
Member

Choose a reason for hiding this comment

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

👍

{ "path": "../url" }
],
"include": [
"src/middlewares/http-v1.js",
"src/middlewares/media-upload.js",
"src/middlewares/namespace-endpoint.js",
"src/middlewares/nonce.js",
"src/middlewares/preloading.js",
"src/middlewares/root-url.js",
"src/middlewares/user-locale.js",
"src/utils/**/*",
"src/types.ts"
"src/**/*",
],
"exclude": [
"**/test/**/*",
]
}