fermyon / spin-js-sdk

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

add router to the SDK #99

Closed karthik2804 closed 1 year ago

karthik2804 commented 1 year ago

Adds a router to the SDK using the itty-router

Usage example of the router

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

const encoder = new TextEncoder()

router.get("/", () => {return {status: 200, body: encoder.encode("default route").buffer}})
router.get("/test", () => {return {status: 200, body: encoder.encode("test route").buffer}})

export const handleRequest: HandleRequest = async function (request: HttpRequest): Promise<HttpResponse> {

    return await router.handle(request)
}

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

karthik2804 commented 1 year ago

The intention behind it is to simplify the handle function. If users import it manually into their app

await router.handle({
    method: request.method,
    url: spin.headers["spin-full-url"]
})

While in the case that it is a part of the SDK as per the conversation with @radu-matei, it simplifies down to

await router.handle(request)

Don't have a really strong opinion on either of the approaches in particular. thoughts?

dicej commented 1 year ago

Thanks for explaining, @karthik2804. I would vote for either adding a handleRequest method to Router that takes an HttpRequest (e.g. using a Proxy) or adding a top-level spinSdk function handleRequest function that takes both a Router and an HttpRequest.

karthik2804 commented 1 year ago

@dicej As long as we include the router in the SDK, we would have to inline the types for the router in the SDK file otherwise, it will introduce a dependency on the itty-package for the types package which is fine I guess.

dicej commented 1 year ago

Is there anything wrong with adding the itty-package dependency to the types package? Are we trying to avoid depending on Itty directly so that we have an option to replace it later? If so, that's fine, just trying to understand the constraints here.

I'm fine with abstracting away the Itty dependency if desired, but I'd still like to avoid the global singleton. Perhaps something like:

import { Router, HttpRequest } from "@fermyon/spin-sdk"
const router = Router()
...
router.handle(request)
karthik2804 commented 1 year ago

We would not be able to do the following

import { Router, HttpRequest } from "@fermyon/spin-sdk"

as that would mean that the @fermyon/spin-sdk has to provide an implementation that with the way the project is structured can provide only type definitions for functions that will be available at runtime.

radu-matei commented 1 year ago

My main ask here is that our handler can be:

export const handleRequest: HandleRequest = async function (request: HttpRequest): Promise<HttpResponse>{    
    return await router.handle(request)
}

I think this is a really good experience (as opposed to manually passing the request objects to the router).

karthik2804 commented 1 year ago

The Router function is now exposed through utils and can now be used as

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

const encoder = new TextEncoder()

let router = utils.Router()

router.get("/hello", () => console.log("reached hello"))

export const handleRequest: HandleRequest = async function (request: HttpRequest): Promise<HttpResponse> {

    await router.handleRequest(request)

    return {
        status: 200,
        headers: {"foo": "bar"},
        body: encoder.encode("Hello from JS-SDK").buffer
    }
}