Skip to content

Commit

Permalink
[kfetch] TypeScript-ify (#20914)
Browse files Browse the repository at this point in the history
In order to make the awesome new kfetch api easier to consume in purely TypeScript projects, and since it's a pretty small module with very few dependencies, I converted it to TypeScript.

Along with kfetch I also started a type definition file for `ui/chrome` that we can extend as we go, but will likely be unnecessary after #19992
  • Loading branch information
Spencer authored Jul 19, 2018
1 parent a091cf0 commit b434652
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 37 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@
"@types/classnames": "^2.2.3",
"@types/eslint": "^4.16.2",
"@types/execa": "^0.9.0",
"@types/fetch-mock": "^5.12.2",
"@types/getopts": "^2.0.0",
"@types/glob": "^5.0.35",
"@types/hapi-latest": "npm:@types/[email protected]",
Expand Down
26 changes: 26 additions & 0 deletions src/ui/public/chrome/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

declare class Chrome {
public addBasePath<T = string>(path: T): T;
}

declare const chrome: Chrome;

export default chrome;
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,8 @@
* under the License.
*/

import fetchMock from 'fetch-mock';
import { kfetch } from './kfetch';

jest.mock('../chrome', () => ({
addBasePath: path => `myBase/${path}`,
addBasePath: (path: string) => `myBase/${path}`,
}));

jest.mock('../metadata', () => ({
Expand All @@ -30,15 +27,14 @@ jest.mock('../metadata', () => ({
},
}));

import fetchMock from 'fetch-mock';
import { kfetch } from './kfetch';

describe('kfetch', () => {
const matcherName = /my\/path/;
const matcherName: any = /my\/path/;

describe('resolves', () => {
beforeEach(() =>
fetchMock.get({
matcher: matcherName,
response: new Response(JSON.stringify({ foo: 'bar' })),
}));
beforeEach(() => fetchMock.get(matcherName, new Response(JSON.stringify({ foo: 'bar' }))));
afterEach(() => fetchMock.restore());

it('should return response', async () => {
Expand Down Expand Up @@ -95,19 +91,16 @@ describe('kfetch', () => {

describe('rejects', () => {
beforeEach(() => {
fetchMock.get({
matcher: matcherName,
response: {
status: 404,
},
fetchMock.get(matcherName, {
status: 404,
});
});
afterEach(() => fetchMock.restore());

it('should throw custom error containing response object', () => {
return kfetch({
pathname: 'my/path',
query: { a: 'b' }
query: { a: 'b' },
}).catch(e => {
expect(e.message).toBe('Not Found');
expect(e.res.status).toBe(404);
Expand Down
41 changes: 25 additions & 16 deletions src/ui/public/kfetch/kfetch.js → src/ui/public/kfetch/kfetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,30 @@
*/

import 'isomorphic-fetch';
import { merge } from 'lodash';
import url from 'url';
import chrome from '../chrome';

// @ts-ignore not really worth typing
import { metadata } from '../metadata';
import { merge } from 'lodash';

class FetchError extends Error {
constructor(res, body) {
constructor(public readonly res: Response, public readonly body?: any) {
super(res.statusText);
this.res = res;
this.body = body;
Error.captureStackTrace(this, FetchError);
}
}

export function kfetch(fetchOptions, kibanaOptions) {
export interface KFetchOptions extends RequestInit {
pathname?: string;
query?: { [key: string]: string | number | boolean };
}

export interface KFetchKibanaOptions {
prependBasePath?: boolean;
}

export function kfetch(fetchOptions: KFetchOptions, kibanaOptions?: KFetchKibanaOptions) {
// fetch specific options with defaults
const { pathname, query, ...combinedFetchOptions } = merge(
{
Expand All @@ -43,7 +52,7 @@ export function kfetch(fetchOptions, kibanaOptions) {
'kbn-version': metadata.version,
},
},
fetchOptions,
fetchOptions
);

// kibana specific options with defaults
Expand All @@ -57,20 +66,20 @@ export function kfetch(fetchOptions, kibanaOptions) {
query,
});

const fetching = new Promise(async (resolve, reject) => {
const fetching = new Promise<any>(async (resolve, reject) => {
const res = await fetch(fullUrl, combinedFetchOptions);

if (!res.ok) {
let body;
try {
body = await res.json();
} catch (err) {
// ignore error, may not be able to get body for response that is not ok
}
return reject(new FetchError(res, body));
if (res.ok) {
return resolve(await res.json());
}

resolve(res.json());
try {
// attempt to read the body of the response
return reject(new FetchError(res, await res.json()));
} catch (_) {
// send FetchError without the body if we are not be able to read the body for some reason
return reject(new FetchError(res));
}
});

return fetching;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@
* under the License.
*/

import { kfetchAbortable } from './kfetch_abortable';

jest.mock('../chrome', () => ({
addBasePath: path => `myBase/${path}`,
addBasePath: (path: string) => `myBase/${path}`,
}));

jest.mock('../metadata', () => ({
Expand All @@ -29,6 +27,8 @@ jest.mock('../metadata', () => ({
},
}));

import { kfetchAbortable } from './kfetch_abortable';

describe('kfetchAbortable', () => {
it('should return an object with a fetching promise and an abort callback', () => {
const { fetching, abort } = kfetchAbortable({ pathname: 'my/path' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
* under the License.
*/

import { kfetch } from './kfetch';
import { kfetch, KFetchKibanaOptions, KFetchOptions } from './kfetch';

type Omit<T, K> = Pick<T, Exclude<keyof T, K>>;

function createAbortable() {
const abortController = new AbortController();
Expand All @@ -29,7 +31,10 @@ function createAbortable() {
};
}

export function kfetchAbortable(fetchOptions, kibanaOptions) {
export function kfetchAbortable(
fetchOptions?: Omit<KFetchOptions, 'signal'>,
kibanaOptions?: KFetchKibanaOptions
) {
const { signal, abort } = createAbortable();
const fetching = kfetch({ ...fetchOptions, signal }, kibanaOptions);

Expand Down
4 changes: 4 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,10 @@
dependencies:
"@types/node" "*"

"@types/fetch-mock@^5.12.2":
version "5.12.2"
resolved "https://registry.yarnpkg.com/@types/fetch-mock/-/fetch-mock-5.12.2.tgz#8c96517ff74303031c65c5da2d99858e34c844d2"

"@types/form-data@^2.2.1":
version "2.2.1"
resolved "https://registry.yarnpkg.com/@types/form-data/-/form-data-2.2.1.tgz#ee2b3b8eaa11c0938289953606b745b738c54b1e"
Expand Down

0 comments on commit b434652

Please sign in to comment.