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

Rule proposal: prefer-static-response-json instead of new Response(JSON.stringify()) #1960

Open
regseb opened this issue Nov 11, 2022 · 7 comments

Comments

@regseb
Copy link
Contributor

regseb commented Nov 11, 2022

Description

Prefer the static Response.json() method instead of new Response(JSON.stringify()).

More info:

Fail

const response = new Response(JSON.stringify({ foo: "bar" }));

Pass

const response = Response.json({ foo: "bar" });
@sindresorhus
Copy link
Owner

The method is quite new, so the rule cannot be in the recommended preset, and we should add a note in the docs about it being quite new.

@fisker
Copy link
Collaborator

fisker commented Nov 15, 2022

This pattern seems rarely used.

https://github.com/search?q=new+Response%28JSON.stringify&type=code&p=1

@silverwind
Copy link
Contributor

silverwind commented Nov 25, 2022

For what purpose are you Response like that? I don't see the use case. Usually, Response.json(), without arguments is used to parse a response, but why would one ever need to construct it?

@regseb
Copy link
Contributor Author

regseb commented Nov 26, 2022

There are two json() methods in the Response class:


In my unit tests, I mock the fetch method to return a specific JSON (examples in Cast Kodi):

it("should return video URL", async function () {
    const stub = sinon.stub(globalThis, "fetch").resolves(new Response(
        JSON.stringify({
            media: [{
                id:          "foo",
                offline_url: "http://bar.com/baz.mp4",
            }],
        }),
    ));

    const file = await scraper.extract("https://watch.pokemon.com/player?id=foo");
    assert.equal(file, "http://bar.com/baz.mp4");

    assert.equal(stub.callCount, 1);
    assert.deepEqual(stub.firstCall.args, [
        "https://www.pokemon.com/api/pokemontv/v2/channels/uk",
    ]);
});

With this new rule, the above code would be in error. The code can be simplified to:

it("should return video URL", async function () {
    const stub = sinon.stub(globalThis, "fetch").resolves(Response.json({
        media: [{
            id:          "foo",
            offline_url: "http://bar.com/baz.mp4",
        }],
    }));

    // ...
});

@silverwind
Copy link
Contributor

I see. Yes I suspected the primary use case for this is mocking. So I'm neutral on this one, don't see it much in demand, thought.

@regseb
Copy link
Contributor Author

regseb commented Dec 8, 2022

Deno says: "This addition to the Fetch specification was specifically designed with server side runtimes in mind." Deno 1.22 Release Notes - New Response.json() static method.

For example:

import { serve } from "https://deno.land/std@0.165.0/http/server.ts";

const port = 8080;

const handler = (request) => {
    const body = { userAgent: request.headers.get("user-agent") ?? "Unknown" };

    // Previously:
    // return new Response(JSON.stringify(body), {
    //     headers: { "content-type": "application/json" },
    // });

    // Now:
    return Response.json(body);
};

await serve(handler, { port });

@silverwind
Copy link
Contributor

silverwind commented Dec 12, 2022

Yeah, for servers that support Response for actually responding, it's definitely useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants