oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
74.1k stars 2.76k forks source link

Bun.serve => Static options have ambiguity #13818

Open vamsaya opened 1 month ago

vamsaya commented 1 month ago

What version of Bun is running?

1.1.28

What platform is your computer?

No response

What steps can reproduce the bug?

Bun.serve({
    static: {
        "/api/health-check": new Response("🟢"),
    },
    async fetch(req) {
        return new Response("Dynamic!");
    },
});

The static routing of Bun is a significant update, but the request methods it faces are too broad. I think it should be specific to the request methods, such as GET POST,Just like below

Bun.serve({
    static: {
        GET: {
            "/old-link": Response.redirect("/new-link", 301),
        },
        POST: {
            "/api/health-check": new Response("🟢"),
        }
    },

    async fetch(req) {
        return new Response("Dynamic!");
    },
});

What is the expected behavior?

No response

What do you see instead?

No response

Additional information

No response

Jarred-Sumner commented 1 month ago

Very reasonable. The main reason we didn’t do this was because of HEAD & OPTIONS, you typically want those handled automatically for GET. And POST for a static route is sort of ambiguous - there’s no data being created since it is static.

vamsaya commented 1 month ago

Very reasonable. The main reason we didn’t do this was because of HEAD & OPTIONS, you typically want those handled automatically for GET. And POST for a static route is sort of ambiguous - there’s no data being created since it is static.

And POST for a static route is sort of ambiguous Semantically POST doesn't seem to be suitable for static, just like downloading a file, which is functionally supported by POST, but few people use it. Therefore, it is up to the developer to decide exactly how to design it.

As you said, it was designed with the specificity of HEAD & OPTIONS in mind.

 static: {
        "/api/health-check": new Response("🟢"),
    }

But when '/api/health-check' is used with GET, POST, PUT...... When you can get the request, it may not be completely semantic. If the same route returns different content for different request methods, the existing static route obviously cannot be implemented, and can only be implemented in the fetch function.

I think it's up to the developer to consider whether to use specific or broad routes.

static: {
    // concrete
    GET: {
        "/old-link": Response.redirect("/new-link", 301),
    },
    POST: {
        "/api/health-check": new Response("🟢"),
    }
    // Broad
    "/any/method":new Response("Hello BunJS")
}

BunJS's static routing is undoubtedly the most exciting feature, requesting the same content, the test data is below. Respond directly in fetch image In fetch, the new URL gets the pathname and responds image static Direct response image

According to the above tests, it is obvious that the performance of static is the most powerful, and it is expected that BunTeam can implement its own routing ecosystem, and it is better to add a pathname to the request of the fetch function, rather than obtaining it through the new URL, which consumes performance.

Here's a simple route I wrote, I don't know if it makes sense. By the way, Bun.Glob is very easy to use, and I really like the Bun native stuff.

const routeObject = {
    "/static/router/name": {
        GET: {
            handler() {
                return new Response("GET: Hello BunJS!");
            }
        },
        POST: {
            handler() {
                return new Response("POST: Hello BunJS!");
            }
        }
    },
    GET: [
        { route: "/user/*", handler() { } }
    ],
    POST: [
        { route: "/api/v{1,2}/isAdmin", handler() { } }
    ]
};

Bun.serve({
    fetch(request) {

        const url = new URL(request.url);

        // static matching
        const router = routeObject[url.pathname]?.[request.method];
        if (router) {
            return router.handler();
        }

        // dynamic matching
        const routes = routeObject[request.method] ?? [];
        for (const router of routes) {
            const glob = new Bun.Glob(router.route);
            if (!glob.match(url.pathname)) continue;
            return router.handler();
        }

        // NotFound
        return new Response("Not Found", { status: 404 });
    }
});