Skip to content

Commit

Permalink
defaultHeaders enhance
Browse files Browse the repository at this point in the history
- `defaultHeaders`: new strategy options - `defaults-set`, `defaults-append` (minor)
- `defaultHeaders`: default strategy now is `set` (major)
- some deps updated (patch)
  • Loading branch information
krutoo committed Dec 8, 2024
1 parent 61cda67 commit 1f84cbd
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 53 deletions.
4 changes: 2 additions & 2 deletions deno.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
},
"imports": {
"@deno/dnt": "jsr:@deno/dnt@^0.41.3",
"@std/expect": "jsr:@std/expect@^1.0.6",
"@std/testing": "jsr:@std/testing@^1.0.3",
"@std/expect": "jsr:@std/expect@^1.0.9",
"@std/testing": "jsr:@std/testing@^1.0.6",
"#fetch": "./src/fetch/mod.ts",
"#response": "./src/response/mod.ts"
},
Expand Down
58 changes: 32 additions & 26 deletions deno.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/fetch/__test__/applyMiddleware.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { it } from '@std/testing/bdd';
import { test } from '@std/testing/bdd';
import { expect } from '@std/expect';
import { applyMiddleware } from '../apply-middleware.ts';
import type { Middleware } from '../types.ts';

it('should apply middleware in the order in which they are passed', async () => {
test('should apply middleware in the order in which they are passed', async () => {
const log: string[] = [];

const foo: Middleware = async (request, next) => {
Expand Down
119 changes: 119 additions & 0 deletions src/middleware/__test__/default-headers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import { test } from '@std/testing/bdd';
import { expect } from '@std/expect';
import { defaultHeaders } from '../default-headers.ts';

test('strategy "set", should be default', async () => {
const middleware = defaultHeaders({
'content-type': 'application/json',
});

const request = new Request('https://foo.bar', {
method: 'POST',
body: JSON.stringify({ foo: 'bar' }),
});

expect(request.headers.get('Content-Type')).toBe('text/plain;charset=UTF-8');

const next = (req: Request) => {
state.request = req;
return new Response('ok');
};

const state: { request: Request | null } = {
request: null,
};

await middleware(request, next);

expect(state.request?.headers.get('Content-Type')).toBe('application/json');
});

test('strategy "append"', async () => {
const middleware = defaultHeaders({
'content-type': 'application/json',
}, {
strategy: 'append',
});

const request = new Request('https://foo.bar', {
method: 'POST',
body: JSON.stringify({ foo: 'bar' }),
});

expect(request.headers.get('Content-Type')).toBe('text/plain;charset=UTF-8');

const next = (req: Request) => {
state.request = req;
return new Response('ok');
};

const state: { request: Request | null } = {
request: null,
};

await middleware(request, next);

expect(state.request?.headers.get('Content-Type')).toBe(
'text/plain;charset=UTF-8, application/json',
);
});

test('strategy "defaults-set"', async () => {
const middleware = defaultHeaders({
'content-type': 'application/json',
}, {
strategy: 'defaults-set',
});

const request = new Request('https://foo.bar', {
method: 'POST',
body: JSON.stringify({ foo: 'bar' }),
});

expect(request.headers.get('Content-Type')).toBe('text/plain;charset=UTF-8');

const next = (req: Request) => {
state.request = req;
return new Response('ok');
};

const state: { request: Request | null } = {
request: null,
};

await middleware(request, next);

expect(state.request?.headers.get('Content-Type')).toBe(
'text/plain;charset=UTF-8',
);
});

test('strategy "defaults-set"', async () => {
const middleware = defaultHeaders({
'content-type': 'application/json',
}, {
strategy: 'defaults-append',
});

const request = new Request('https://foo.bar', {
method: 'POST',
body: JSON.stringify({ foo: 'bar' }),
});

expect(request.headers.get('Content-Type')).toBe('text/plain;charset=UTF-8');

const next = (req: Request) => {
state.request = req;
return new Response('ok');
};

const state: { request: Request | null } = {
request: null,
};

await middleware(request, next);

expect(state.request?.headers.get('Content-Type')).toBe(
'application/json, text/plain;charset=UTF-8',
);
});
6 changes: 3 additions & 3 deletions src/middleware/__test__/retry.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterAll, beforeAll, it } from '@std/testing/bdd';
import { afterAll, beforeAll, test } from '@std/testing/bdd';
import { expect } from '@std/expect';
import { applyMiddleware, configureFetch } from '#fetch';
import { retry } from '../retry.ts';
Expand All @@ -22,7 +22,7 @@ afterAll(async () => {
await server.shutdown();
});

it('Should fetch resource until it returns success response)', async () => {
test('Should fetch resource until it returns success response)', async () => {
const fetch = configureFetch(globalThis.fetch, applyMiddleware(retry({ count: 10 })));

expect(serverState.counter).toBe(3);
Expand All @@ -37,7 +37,7 @@ it('Should fetch resource until it returns success response)', async () => {
await dump(response);
});

it('Should break fetch loop when attempt limit is reached', async () => {
test('Should break fetch loop when attempt limit is reached', async () => {
const fetch = configureFetch(globalThis.fetch, applyMiddleware(retry({ count: 2 })));

serverState.counter = 5;
Expand Down
60 changes: 45 additions & 15 deletions src/middleware/default-headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ import type { Middleware } from '#fetch';
/** Options of default headers middleware. */
export interface DefaultHeadersOptions {
/**
* How to add header to request headers.
* - "set" - headers will be added using "set" method
* - "append" - headers will be added using "append" method
* How to add headers to request headers.
* - "set" - `defaults` will be added to `request.headers` using "set" method
* - "append" - `defaults` will be added to `request.headers` using "append" method
* - "defaults-set" - `request.headers` will be added to `defaults` using "set" method
* - "defaults-append" - `request.headers` will be added to `defaults` using "append" method
*/
strategy?: 'set' | 'append';
strategy?: 'set' | 'append' | 'defaults-set' | 'defaults-append';
}

/**
Expand All @@ -17,24 +19,52 @@ export interface DefaultHeadersOptions {
*/
export function defaultHeaders(
defaults: HeadersInit,
{ strategy = 'append' }: DefaultHeadersOptions = {},
{ strategy = 'set' }: DefaultHeadersOptions = {},
): Middleware {
return (request, next) => {
/**
* Previously, there was a different approach here:
* headers were created based on "defaults" argument,
* then headers from the request were added to them.
*
* This was done so that the "default headers" were
* truly default and were overridden by what was set by
* the developer in the request itself.
*
* But it didn't work well because browser always
* had the "Content-Type" header set by default,
* which always overridden the option that was in the
* middleware factory arguments.
*
* To fix this, `strategy` option is introduced.
*/

// IMPORTANT: for avoid mutate request, just create new Headers and Request here
const headers = new Headers(request.headers);
const headers = new Headers(strategy.startsWith('defaults-') ? defaults : request.headers);

/*
Previously, there was a different approach here: headers were created based on "defaults" argument, then headers from the request were added to them.
if (strategy === 'defaults-set') {
request.headers.forEach((value, key) => {
headers.set(key, value);
});
}

This was done so that the "default headers" were truly default and were overridden by what was set by the developer in the request itself.
if (strategy === 'defaults-append') {
request.headers.forEach((value, key) => {
headers.append(key, value);
});
}

But it didn't work well because browser always had the "Content-Type" header set by default, which always overridden the option that was in the middleware factory arguments.
if (strategy === 'set') {
new Headers(defaults).forEach((value, key) => {
headers.set(key, value);
});
}

To fix this, default headers are now added to the request headers
*/
new Headers(defaults).forEach((value, key) => {
headers[strategy](key, value);
});
if (strategy === 'append') {
new Headers(defaults).forEach((value, key) => {
headers.append(key, value);
});
}

return next(new Request(request, { headers }));
};
Expand Down
Loading

0 comments on commit 1f84cbd

Please sign in to comment.