fermyon / spin-js-sdk

https://developer.fermyon.com/spin/javascript-components
Apache License 2.0
52 stars 18 forks source link

add new handler signature `handler(req, res)` #116

Closed karthik2804 closed 1 year ago

karthik2804 commented 1 year ago

This PR introduces the following additional signature for the handler.

import { EventHandler, HttpRequest, HttpResponse } from "spin-sdk"

export const eventHandler: EventHandler = async function (request: HttpRequest, response: ResponseObject): Promise<void> {

    response.status(200).header("content-type", "text/html").body("Chaining works")
}

Question: Is the function name eventHandler satisfactory or are there any other suggestions?

Signed-off-by: karthik Ganeshram karthik.ganeshram@fermyon.com

karthik2804 commented 1 year ago

Integrating the routing and request handling might opinionate things a little. Since we already have a router, users can choose how they would like to route things.

I chose eventHandler because it sounds like a generic name. We could potentially use httpEventHandler or handleHttpEvent to be a bit more specific. Thoughts?

itowlson commented 1 year ago

This may be unfeasible given the internals, but could you keep the same handleRequest name and be more flexible with the type? (In TypeScript terms, a union type.)

karthik2804 commented 1 year ago

@itowlson, based on the number of arguments the user-defined function expects, I think that is a possibility. It could become confusing to the user. I do not have a strong opinion about this though.

radu-matei commented 1 year ago

The goal of building a JavaScript SDK is to make it easy for JavaScript developers to build Spin applications — not necessarily for people familiar with Spin SDKs.

If we can make it so the signature (in particular) and function name make it easier for developers to map their existing knowledge to building a Spin application, I would prioritize that over being consistent with other Spin SDKs (particularly considering we already have the handleRequest function that is consistent with other Spin SDKs).

handler(req, res) sounds good to me.

This touches a bit on the broader aspect of how we treat consistency across SDKs, and I would make the same argument as here — we want to make it so that people familiar with how to write an HTTP handler in language X find our SDK for language X familiar; it wouldn't matter to them that the SDKs happen to be consistent with one another.

karthik2804 commented 1 year ago

As suggested, I agree with the argument and updated the function name to handler.