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

[Improvement]: Remove content type from DELETE and PUT methods by default #79

Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export const APPLICATION_CONTENT_TYPE = 'application/';

export const APPLICATION_JSON = APPLICATION_CONTENT_TYPE + 'json';
export const CHARSET_UTF_8 = 'charset=utf-8';
export const CONTENT_TYPE = 'Content-Type';

export const UNDEFINED = 'undefined';
Expand Down
38 changes: 36 additions & 2 deletions src/request-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
FetcherConfig,
FetcherInstance,
Logger,
HeadersObject,
} from './types/request-handler';
import type {
BodyPayload,
Expand All @@ -37,6 +38,7 @@ import {
ABORT_ERROR,
APPLICATION_JSON,
CANCELLED_ERROR,
CHARSET_UTF_8,
CONTENT_TYPE,
GET,
HEAD,
Expand All @@ -56,7 +58,6 @@ const defaultConfig: RequestHandlerConfig = {
headers: {
Accept: APPLICATION_JSON + ', text/plain, */*',
'Accept-Encoding': 'gzip, deflate, br',
[CONTENT_TYPE]: APPLICATION_JSON + ';charset=utf-8',
},
retry: {
delay: 1000,
Expand Down Expand Up @@ -165,6 +166,35 @@ export function createRequestHandler(
}
};

/**
* Sets the Content-Type header to 'application/json;charset=utf-8' if needed based on the method and body.
*
* @param headers - The headers object where Content-Type will be set.
* @param method - The HTTP method (e.g., GET, POST, PUT, DELETE).
* @param body - Optional request body to determine if Content-Type is needed.
*/
const setContentTypeIfNeeded = (
headers: HeadersInit,
method: string,
body?: unknown,
): void => {
if (!body && ['PUT', 'DELETE'].includes(method)) return;
MattCCC marked this conversation as resolved.
Show resolved Hide resolved

const contentTypeValue = APPLICATION_JSON + ';' + CHARSET_UTF_8;

if (headers instanceof Headers) {
if (!headers.has(CONTENT_TYPE)) {
headers.set(CONTENT_TYPE, contentTypeValue);
}
} else if (
typeof headers === 'object' &&
MattCCC marked this conversation as resolved.
Show resolved Hide resolved
!Array.isArray(headers) &&
!headers[CONTENT_TYPE]
) {
headers[CONTENT_TYPE] = contentTypeValue;
}
};

/**
* Build request configuration
*
Expand Down Expand Up @@ -202,6 +232,10 @@ export function createRequestHandler(
body = explicitBodyData;
}

const headers = getConfig<HeadersObject>(requestConfig, 'headers');

setContentTypeIfNeeded(headers, method, body);

// Native fetch compatible settings
const isWithCredentials = getConfig<boolean>(
requestConfig,
Expand Down Expand Up @@ -236,8 +270,8 @@ export function createRequestHandler(
credentials,
body,
method,

url: baseURL + urlPath,
headers,
};
};

Expand Down
68 changes: 65 additions & 3 deletions test/request-handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type {
RequestHandlerReturnType,
} from '../src/types/request-handler';
import { fetchf } from '../src';
import { ABORT_ERROR } from '../src/constants';
import { ABORT_ERROR, APPLICATION_JSON, CHARSET_UTF_8 } from '../src/constants';
import { ResponseErr } from '../src/response-error';

jest.mock('../src/utils', () => {
Expand All @@ -26,6 +26,7 @@ const fetcher = {

describe('Request Handler', () => {
const apiUrl = 'http://example.com/api/';
const contentTypeValue = APPLICATION_JSON + ';' + CHARSET_UTF_8;
const responseMock = {
data: {
test: 'data',
Expand Down Expand Up @@ -64,11 +65,13 @@ describe('Request Handler', () => {
const headers = {
Accept: 'application/json, text/plain, */*',
'Accept-Encoding': 'gzip, deflate, br',
'Content-Type': 'application/json;charset=utf-8',
'Content-Type': contentTypeValue,
};

beforeAll(() => {
requestHandler = createRequestHandler({});
requestHandler = createRequestHandler({
headers,
});
});

it('should not differ when the same request is made', () => {
Expand Down Expand Up @@ -270,6 +273,65 @@ describe('Request Handler', () => {
});
});

describe('request() Content-Type', () => {
let requestHandler: RequestHandlerReturnType;
const contentTypeValue = 'application/json;charset=utf-8';

beforeEach(() => {
requestHandler = createRequestHandler({});
});

afterEach(() => {
jest.clearAllMocks();
});

describe.each([
{ method: 'DELETE', body: undefined, expectContentType: false },
{ method: 'PUT', body: undefined, expectContentType: false },
{ method: 'DELETE', body: { foo: 'bar' }, expectContentType: true },
{ method: 'PUT', body: { foo: 'bar' }, expectContentType: true },
{ method: 'POST', body: undefined, expectContentType: true },
{ method: 'GET', body: undefined, expectContentType: true },
])(
'$method request with body: $body',
({ method, body, expectContentType }) => {
it(
expectContentType
? 'should set Content-Type when body is provided or method requires it'
: 'should not set Content-Type when no body is provided for DELETE or PUT',
() => {
const result = requestHandler.buildConfig(apiUrl, { method, body });
if (expectContentType) {
expect(result.headers).toHaveProperty(
'Content-Type',
contentTypeValue,
);
} else {
expect(result.headers).not.toHaveProperty('Content-Type');
}
},
);
},
);

describe.each(['DELETE', 'PUT'])(
'%s method with custom Content-Type',
(method) => {
it(`should keep custom Content-Type for ${method} method`, () => {
const customContentType = 'application/x-www-form-urlencoded';
const result = requestHandler.buildConfig(apiUrl, {
method,
headers: { 'Content-Type': customContentType },
});
expect(result.headers).toHaveProperty(
'Content-Type',
customContentType,
);
});
},
);
});

describe('request()', () => {
beforeEach(() => {
jest.useFakeTimers();
Expand Down
Loading