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

feat: typed +server.ts responses with fetch #11108

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AlbertMarashi
Copy link

@AlbertMarashi AlbertMarashi commented Nov 23, 2023

Attempting to solve

Approach

/api/[post]/+server.ts

import { typed_json } from '@sveltejs/kit';

export async function GET({ params }) {
    return typed_json({ post_id: params.post })
}

utils

interface TypedResponse<A> extends Response {
    json(): Promise<A>;
}

// A typed json wrapper (this could probably just replace `json`
function typed_json<A>(data: A): TypedResponse<A> {
    return json(data)
}

+page.ts

import { api_fetch } from "$api";

export async function load({ fetch }) {
    const data = await fetch("/api/[post]", {
        method: "GET",
        params: { post: "123" }
    });

    return {
        text: data.post_id // typed as a string
    }
}

+page.svelte

<script lang="ts">
// a typed custom version of fetch available inside of components
import api_fetch from "$api"
</script>

Theory

Using function overloading in typescript we can overload the fetch or custom fetch method with type info that gives us information about available APIs, return types and parameters.

This system could also be extended to provide typed inputs for the body parameter in fetch as well as supporting typed gotos redirects and more

The custom fetch provided in the load functions and other places would more or less reflect the structure of the native fetch except with a few changes:

  1. The URL is parsed, to obtain areas to inject the provided params
  2. Some of the RequestInit properties are overrided to force things such as method: "GET" as well as providing an additional parameter called params

example

type Methods = "GET" | "POST" | "UPDATE" | "DELETE"
type FetchOptions<Params = never, Method extends Methods = Methods> =
    Parameters<typeof fetch>[1] & {
        params: Params,
        method: Method
    }
    
 
export async function api_fetch(url: "/api/posts/[post]", options?: FetchOptions<ApiPostsPostParamParams, "GET">): Promise<TypedResponse<{ foo: string }>>;
export async function api_fetch(url: string, options?: Parameters<typeof fetch>[1] | undefined): Promise<any> {
    // parse the url
    // inject the params in the correct locations
    // call the fetch method
}

Proof of concept example

interface TypedResponse<O> extends Response {
    json(): Promise<O>
}

type ApiPostsPostParamParams = {
    post: string
}

type Methods = "GET" | "POST" | "UPDATE" | "DELETE"

type FetchOptions<Params = never, Method extends Methods = Methods> =
    Parameters<typeof fetch>[1] & {
        params: Params,
        method: Method
    }


// we declare these inside of `.svelte-kit/types/src/api/posts/[post]/$types.d.ts`
type RouteId1 = "/api/posts/[post]"
type RouteParams1 = { post: string }
type Route1Output = { abc: string }

declare module "$api" {
    export function api_fetch(url: RouteId1, options?: FetchOptions<RouteParams1, "GET">): Promise<TypedResponse<Route1Output>>;
}

// we declare these inside of `.svelte-kit/types/src/api/comments/[comments]/$types.d.ts`
type RouteId2 = "/api/comments/[comments]"
type RouteParams2 = { comment: string }
type Route2Output = { xyz: string }

declare module "$api" {
    export function api_fetch(url: RouteId2, options?: FetchOptions<RouteParams2, "GET">): Promise<TypedResponse<Route2Output>>;
}
import { api_fetch } from "$api"

// no errors & correct typing
const data = await api_fetch("/api/posts/[post]", {
    method: "GET",
    params: {
        post: "post-id"
    }
})

I am posting this as a draft PR to gain feedback from the community and contributors.

Some unresolved matters:


Related PRs

Copy link

changeset-bot bot commented Nov 23, 2023

⚠️ No Changeset found

Latest commit: 21b15e4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@AlbertMarashi
Copy link
Author

For typing request body params we could expose a type available to +server.ts files, and then extract these types and use them in the FetchOptions helper to provide typing for the inputs of an API call.

interface TypedRequest<T = any> {
    json(): Promise<T>
}

interface TypedEvent<T = any> {
    request: TypedRequest<T>
}

export async function GET({ request }: TypedEvent<{ foo: string }>) {
    let { foo } = await request.json()
}

Not sure if there's an easier way to do this, but it feels a lot more explicit than doing

let { foo } = await request.json() as { foo: string }

@difanta
Copy link

difanta commented Dec 3, 2023

Hi, after considering both my proposal in #9938 and yours here my opinion is a little divided. I believe that SvelteKit's mission to make everything accessible to new users, simple to use and implicitly typed is very important.

I liked the way my proposal handles things, because it was a change that everyone could benefit from without changing a single line of code. Though it turned out to be a pain to manage, with extremely complicated Types and trouble overall to represent all the possible fetch cases with one or two overloads.

I also like your proposal which feels a lot natural and leads to code similar to tRPC without requiring a lot of code changes. My only worry is introducing a function like api_fetch which is basically identical to fetch in its behaviour, but exists with the sole purpose of enforcing types on request (body and path) and on the response. I really liked the simplicity of using the same fetch function for everything.

In an ideal world I would choose the typed fetch function (my proposal), but from a user perspective of understanding errors and the types behind the function it is very complicated, and even more complicated is to maintain it in the future.

Overall yours seems the most feasible one. How about redirect and goto functions? Should they be rewritten too in the same way?

Maybe for fetch, redirect and goto we could provide the base function for non relative paths (ex. Https://...) While for root relative paths (/...) we could provide your api_fetch with the path parameters in the request by overloading the base fetch function. And the same goes for redirect and goto.
This way the function would only be one but its signature would vary if the path is a relative api path or not.

@Shaun-Regenbaum
Copy link

Is this going to be accepted eventually? Can we not just roll the behaviour of the would be 'api-fetch' into the standard sveltekit fetch? It already has special behaviours (as when allowing local routes to autocomplete to the full route).

@AlbertMarashi
Copy link
Author

AlbertMarashi commented Dec 4, 2023

Is this going to be accepted eventually? Can we not just roll the behaviour of the would be 'api-fetch' into the standard sveltekit fetch? It already has special behaviours (as when allowing local routes to autocomplete to the full route).

Yes I believe it would be possible to roll the behavior into the standard sveltekit fetch, and do a check on whether the params argument is there / inject them into the URL template

Overall yours seems the most feasible one. How about redirect and goto functions? Should they be rewritten too in the same way?

Yes, I believe it would be possible to support this as well, to ensure that goto's and redirects are valid.

Additionally this change should be backwards compatible, and it will still be possible to manually call the API routes with no typing/params provided in the API call

With fetch, the pseudo algorithm could work similarly to this

fn fetch(url, options)
  if options?.params
    parse the url
    inject the params
    set url to new url
  call the native fetch api

It should be also possible to import the svelte custom fetch into the context of components too

@AlbertMarashi
Copy link
Author

@Rich-Harris thoughts on this proposal?

@thomasmol
Copy link

feels like this is would be the last major addition to make a sveltekit project fully typesafe all around, especially with the new typesafety in component markup with svelte 5.0

big fan of this 👍

@Shaun-Regenbaum
Copy link

I would like to add my support here, I am fan of this proposal over #9938, I think the extra complexity is worthwhile to get a more tRPC like experience.

@AlbertMarashi
Copy link
Author

AlbertMarashi commented Mar 19, 2024

@Shaun-Regenbaum

I would like to add my support here, I am fan of this proposal over #9938, I think the extra complexity is worthwhile to get a more tRPC like experience.

The typing in that becomes hell as the author mentioned because typescript doesn't have Regex types.

Though it turned out to be a pain to manage, with extremely complicated Types and trouble overall to represent all the possible fetch cases with one or two overloads.

It just makes sense to use URLs that match the path templates of pages and then using a params field in the request options imo

export async function load({ fetch }) {
    const data = await fetch("/api/[post]", {
        method: "GET",
        params: { post: "123" }
    });

    return {
        text: data.post_id // typed as a string
    }
}

would be functionally equivalent to this (minus the types)

export async function load({ fetch }) {
    const data = await fetch("/api/123", {
        method: "GET"
    });

    return {
        text: data.post_id // untyped
    }
}

@AlbertMarashi
Copy link
Author

AlbertMarashi commented Mar 29, 2024

Something like this should be possible to achieve via this approach, here I am using Zod library as an example for input validation

/api/xyz/+server.ts

import { z } from "zod"

const api_inputs = z.object({
   foo: z.string()
})

export async function POST(
  { request }: TypedEvent<z.infer<typeof api_inputs>>
) { // function return type inferred
   const data = api_inputs.parse(await request.json()) // typed & validated input
   
   return json({
      baz: "bar"
   }) // return type validated as TypedResponse<{ baz: "bar" }>
}

client side code

let res = await fetch("/api/xyz", {
   method: "POST",
   body: {
     foo: "hello world"
   } // this body would be type-checked
}) // TypedResponse<{ baz: "bar"}>

const data = await res.json() //  typed as { baz: "bar" }

@PuruVJ
Copy link
Contributor

PuruVJ commented Mar 29, 2024

In favor of making this the default behavior of fetch, but in a major release, as this could cause sudden type errors in some existing code.

@benmccann benmccann added this to the 3.0 milestone Mar 29, 2024
@eltigerchino eltigerchino changed the title Draft: Typed +server.ts responses with fetch feat: typed +server.ts responses with fetch Mar 29, 2024
@AlbertMarashi
Copy link
Author

@PuruVJ I'm not so sure that this is actually a breaking change.

We would want to create a $api module in order to not override the browser implementation of fetch

The $api fetch module would return instances of type TypedResponse<T> = Response

@benmccann benmccann removed this from the 3.0 milestone Mar 30, 2024
@hmnd
Copy link
Contributor

hmnd commented May 2, 2024

Understand the team is likely very busy with Svelte 5 RC stuff, but would love to get this merged! Are there any further blockers?

@AlbertMarashi
Copy link
Author

AlbertMarashi commented May 6, 2024

@hmnd yeah, very little is developed in terms of the actual PR, but I think there's a solid theoretical direction for where to go. Might need some help figuring out how we want to structure this in the code, and the right modules i'll need to modify to get this system implemented

  • eg: How to actually add the $api module with the function definition so that it can be imported into svelte projects, similar to $page store or other $ stores, essentially where to put the real function implementation such that it is accessible by projects

@AlbertMarashi
Copy link
Author

AlbertMarashi commented May 7, 2024

Some additional benefits/advantages to this is that:

  1. We could type-check/catch invalid fetch URLs since we can detect if the URL exists and if someone tries to include params object with a non-existent URL, that would appear as a type error (this would not be the case with using the default untyped version of fetch)
  2. Similarly, we could extend the goto function with similar behavior to ensure all page navigations go to an endpoint exists
  3. Naturally, we also get typed responses to further help guarantee that the code that follows the call is type checked should the return interfaces be adapted

@huseeiin
Copy link
Contributor

does this work in client side too like in event bindings?

@AlbertMarashi
Copy link
Author

AlbertMarashi commented Sep 13, 2024

does this work in client side too like in event bindings?

Could you elaborate?

Edit

If you mean like a fetch inside of a on:click svelte javascript expression, then yes, typing should still work.

@huseeiin
Copy link
Contributor

does this work in client side too like in event bindings?

Could you elaborate?

Edit

If you mean like a fetch inside of a on:click svelte javascript expression, then yes, typing should still work.

yes. that's what i meant. nice.

@mingsterism
Copy link

any updates on this ticket guys?

@eltigerchino eltigerchino linked an issue Dec 4, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants