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.

Sign up for GitHub

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

Jump to bottom

feat: add static Response.json #1670

Merged
merged 2 commits into from Nov 10, 2022
Merged

feat: add static Response.json #1670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e018ea
feat: add static Response.json
KhafraDev Nov 8, 2022
89480fa
fix: set content-type if it doesn't exist properly
KhafraDev Nov 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions @types/index.d.ts
View file
Open in desktop
Expand Up @@ -196,6 +196,7 @@ export class Response extends BodyMixin {

static error(): Response;
static redirect(url: string, status?: number): Response;
static json(data: any, init?: ResponseInit): Response;
}

export class FetchError extends Error {
Expand Down
5 changes: 5 additions & 0 deletions @types/index.test-d.ts
View file
Open in desktop
Expand Up @@ -93,6 +93,11 @@ async function run() {

expectType<Response>(Response.redirect('https://google.com'));
expectType<Response>(Response.redirect('https://google.com', 301));

expectType<Response>(Response.json({foo: 'bar'}));
expectType<Response>(Response.json({foo: 'bar'}, {
status: 301
}));
}

run().finally(() => {
Expand Down
19 changes: 19 additions & 0 deletions src/response.js
View file
Open in desktop
Expand Up @@ -124,6 +124,25 @@ export default class Response extends Body {
return response;
}

static json(data = undefined, init = {}) {
Copy link
Member

Choose a reason for hiding this comment

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

I think that using init = {} and assuming init is an object further down in the function will make the following code crash:

Response.json({ hej: 1 }, null)

But in Chrome that works:

Screenshot 2022-11-09 at 09 22 54

Copy link
Contributor Author

Choose a reason for hiding this comment

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

node-fetch doesn't seem to do much argument validation, I followed other areas of the code.

For example when constructing a new response:

new Response('', null)

throws an error when it shouldn't

new Response('', 3)

works when it shouldn't (same with booleans, etc)

KhafraDev marked this conversation as resolved.
Show resolved Hide resolved
const body = JSON.stringify(data);

if (body === undefined) {
throw new TypeError('data is not JSON serializable');
}

const headers = new Headers(init && init.headers);

if (!headers.has('content-type')) {
headers.set('content-type', 'application/json');
}

return new Response(body, {
...init,
headers
});
}

get [Symbol.toStringTag]() {
return 'Response';
}
Expand Down
27 changes: 27 additions & 0 deletions test/main.js
View file
Open in desktop
Expand Up @@ -2281,6 +2281,33 @@ describe('node-fetch', () => {
const res = await fetch(url);
expect(res.url).to.equal(`${base}m%C3%B6bius`);
});

it('static Response.json should work', async () => {
const response = Response.json({foo: 'bar'});
expect(response.status).to.equal(200);
expect(response.headers.get('content-type')).to.equal('application/json');
expect(await response.text()).to.equal(JSON.stringify({foo: 'bar'}));

const response1 = Response.json(null, {
status: 301,
statusText: 'node-fetch',
headers: {
'Content-Type': 'text/plain'
}
});

expect(response1.headers.get('content-type')).to.equal('text/plain');
expect(response1.status).to.equal(301);
expect(response1.statusText).to.equal('node-fetch');

const response2 = Response.json(null, {
headers: {
'CoNtEnT-TypE': 'text/plain'
}
});

expect(response2.headers.get('content-type')).to.equal('text/plain');
});
});

describe('node-fetch using IPv6', () => {
Expand Down