Overall feedback

[plckr]

trafficstars

Hey, I'm looking for ways to implement openapi into sveltekit, I found this repository and I'm currently testing.

I find it pretty interesting, nice work!

There is specially one thing that bothers me, is that you suggest to duplicate routes in different locations. Inside routes and inside a custom folder (you name it api). I wonder why you implemented this way. Doesn't it make more sense to have the routes into a single location only?

For example: Having a catch all into routes/api/[...route] and pointing all requests into the api.

// routes/api/[...route]/+server.ts

import api from '$api';
import type { RequestHandler } from './$types';

export const GET: RequestHandler = (evt) => api.handle(evt);
export const POST: RequestHandler = (evt) => api.handle(evt);
export const PUT: RequestHandler = (evt) => api.handle(evt);
export const DELETE: RequestHandler = (evt) => api.handle(evt);
export const OPTIONS: RequestHandler = (evt) => api.handle(evt);

and then control the routes in a different or the same location.

---

Another thing I just noticed is that this library isn't capable to deal with group routes E.g.: /routes/(protected)/api/some-route The example above will be read from the library as (protected)/api/some-route

I tried adding a new replace into the parse_module function and it seems to work well At this line, add .replace(/\(.+\)\//g, "")

---

For last, don't you find it easier to export an object from each route/method instead of exports individual variables? For someone that isn't aware of the library, it's more difficult to remember each variable to export. My suggestion would be something like this:

import api from '$api';

// a function is used for type-safety purposes
export default api.defineEndpoint({
  query: z.object({
    password: z.string().optional(),
  }),
  param: z.object({
    id: z.string(),
  }),
  input: z.object({
    title: z.string(),
    content: z.string(),
    author: z.string(),
  }),
  output: z.object({
    id: z.string(),
    title: z.string(),
    content: z.string(),
    author: z.string(),
    date: z.string(),
  }) satisfies z.ZodSchema<Post>,
  error: {
    404: error(404, "Post not found"),
    403: error(403, "Forbidden"),
  },
  endpoint: async (param) => {
    const post = posts.get(param.id);
  
    if (!post) {
      throw Error[404];
    }
  
    if (post.password && post.password !== param.password) {
      throw Error[403];
    }
  
    post.title = param.title;
    post.content = param.content;
    post.author = param.author;
  
    return post;
  }
})

I'm able to fork and submit a pull request if you wish, I had like to discuss first.

[JacobLinCool]

Hi @plckr,

Thanks for your feedback!

I agree with you that using a single wildcard route like routes/api/[...route] would simplify things, but it requires reimplementing the router logic in SvelteKit, I've try to do this before, but it seems that SvelteKit doesn't export the resolve function for their router.

---

I saw your PR. Thanks!

---

I agree with exporting an object for better readability and usability. The Endpoint class is designed for this purpose. Here's an example from the README:

// file: src/api/post/[...id]/PUT.ts
import { Endpoint, z, error } from "sveltekit-api";
import { posts, type Post } from "../../db.js";

const Query = z.object({
  password: z.string().optional(),
});

const Param = z.object({
  id: z.string(),
});

const Input = z.object({
  title: z.string(),
  content: z.string(),
  author: z.string(),
});

const Output = z.object({
  id: z.string(),
  title: z.string(),
  content: z.string(),
  author: z.string(),
  date: z.string(),
}) satisfies z.ZodSchema<Post>;

const Error = {
  404: error(404, "Post not found"),
  403: error(403, "Forbidden"),
};

export default new Endpoint({ Param, Query, Input, Output, Error }).handle(async (param) => {
  const post = posts.get(param.id);

  if (!post) {
    throw Error[404];
  }

  if (post.password && post.password !== param.password) {
    throw Error[403];
  }

  post.title = param.title;
  post.content = param.content;
  post.author = param.author;

  return post;
});

Since the Endpoint is typed, users can get the intellisense hint like what you've purposed.

[plckr]

1. Now I understand what problems you may face, this is not straightforward.
2. The problem I see with your example, is that you need to export individually every input for the Endpoint, with the defineEndpoint everything is exported, including the Endpoint class.

I'll probably make a PR just to show what I mean, and then we discuss it. I understand that this points isn't a "must have" but simplifies the consumer code

[JacobLinCool]

For the second point, that's definitely my mistake regarding the misleading example code. In the earlier days before the Endpoint was introduced, everything needed to be exported. Then Endpoint was introduced and brought better type hints. Now, only one of them needs to be exported, not both. But I forgot to remove the export for the other inputs.

And for the first part, I think any help is appreciated because I don't have any idea on that right now. What do you think if we have a partially supported route resolver first before we can get the standard one from SvelteKit?

[plckr]

Ah, nice that it's not needed to export those variables!

I had some brainstorming on how it could be improved, I'll paste it here so that you can look at it. I don't know the entire code as much as you do, so probably it'll enlight other ideas on you.

type DefineEndpointOptions<P extends z.ZodType, O extends z.ZodType> = {
  param?: P;
  output?: O;
  handle: (
    parameters: z.infer<P>,
    req: import("./$types.js").RequestEvent,
  ) => Promise<z.infer<O>> | z.infer<O>;
};

class API {
  defineEndpoint<
    P extends z.ZodType = z.ZodObject<Record<never, never>>,
    O extends z.ZodType = z.ZodObject<Record<never, never>>,
  >({ param, ...options }: DefineEndpointOptions2<P, O>) {
    async function __sveltekit_api__handler(evt: import("./$types.js").RequestEvent) {
      const paramValidator = param || z.object({});
      const params = paramValidator.safeParse(evt.params);

      const result = await options.handle(params.success ? params.data : {}, evt);

      return json(result);
    }

    // will return "__sveltekit_api__handler"
    // useful to find openapi routes (?) when iterating files from vite
    console.log(__sveltekit_api__handler.name);

    return __sveltekit_api__handler;
  }
}

and then in routes/api/[param]/+server.ts

export const GET = api.defineEndpoint({
  param: Param,
  output: Output,
  handle: async (parameters, req) => {
    return { status: "ok" };
  },
});

With this, we keep the logic only in one place, the api routes, and don't have to duplicate routings

does this makes any sense? let me know what do you think

[plckr]

additional information:

const modules = import.meta.glob("/src/routes/**/+server.ts");

for (const path in modules) {
	modules[path]().then((mod) => {
		console.log("mod name", mod.GET.name);
	});
}

outputs:

mod name GET
mod name GET
mod name GET
mod name GET
mod name GET
mod name __sveltekit_api__handler
mod name GET

So, it's easier to track which routes are generated from the library or not