Skip to content

Commit

Permalink
fix(http-client): Rework application/json header
Browse files Browse the repository at this point in the history
Before, I was adding it as a default to be overridden, but this didn't match the previous behavior closely enough. The old behavior was to return a Blob from `json()` with `content-type: application/json`, which the fetch client would automatically pick up if no header was present. I've removed the default header and instead added a check when there is no content-type header and the body is valid json. To check the json I use `JSON.parse`, which might have performance issues on very big json objects, but unlikely to cause trouble for anyone.

closes #90
  • Loading branch information
davismj committed Jan 26, 2018
1 parent 180fbfa commit 946273a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
23 changes: 16 additions & 7 deletions src/http-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ export class HttpClient {
if (typeof fetch === 'undefined') {
throw new Error('HttpClient requires a Fetch API implementation, but the current environment doesn\'t support it. You may need to load a polyfill such as https://github.com/github/fetch');
}

// Use application/json as the default content-type.
this.defaults = {
headers: { 'content-type': 'application/json' }
};
}

/**
Expand Down Expand Up @@ -174,8 +169,12 @@ function buildRequest(input, init) {
requestContentType = new Headers(requestInit.headers).get('Content-Type');
request = new Request(getRequestUrl(this.baseUrl, input), requestInit);
}
if (!requestContentType && new Headers(parsedDefaultHeaders).has('content-type')) {
request.headers.set('Content-Type', new Headers(parsedDefaultHeaders).get('content-type'));
if (!requestContentType) {
if (new Headers(parsedDefaultHeaders).has('content-type')) {
request.headers.set('Content-Type', new Headers(parsedDefaultHeaders).get('content-type'));
} else if (body && isJSON(body)) {
request.headers.set('Content-Type', 'application/json');
}
}
setDefaultHeaders(request.headers, parsedDefaultHeaders);
if (body && Blob.prototype.isPrototypeOf(body) && body.type) {
Expand Down Expand Up @@ -222,6 +221,16 @@ function applyInterceptors(input, interceptors, successName, errorName, ...inter
}, Promise.resolve(input));
}

function isJSON(str) {
try {
JSON.parse(str);
}
catch (err) {
return false;
}
return true;
}

function identity(x) {
return x;
}
Expand Down
2 changes: 1 addition & 1 deletion test/http-client.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ describe('HttpClient', () => {

it('uses default content-type header', (done) => {
fetch.and.returnValue(emptyResponse(200));
let contentType = 'application/json;charset=UTF-8';
let contentType = 'application/octet-stream';
client.defaults = { method: 'post', body: '{}', headers: { 'content-type': contentType } };

client.fetch('path')
Expand Down

0 comments on commit 946273a

Please sign in to comment.