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

Allow defining custom API routes (http) #268

Closed
Martinsos opened this issue Jun 28, 2021 · 14 comments · Fixed by #1043
Closed

Allow defining custom API routes (http) #268

Martinsos opened this issue Jun 28, 2021 · 14 comments · Fixed by #1043
Assignees
Labels
enhancement New feature or request

Comments

@Martinsos
Copy link
Member

Right now dev can define operations (actions, queries) which are then consumed from client via RPC that works via http.
However, they can't define custom http API roues at the moment! They might need this, so we should offer them support for this.

This basically means they should be able to drop one level of abstraction below the operations and directly define http routes (in Express right now).

@Martinsos Martinsos added the enhancement New feature or request label Jun 28, 2021
@Martinsos Martinsos self-assigned this Jul 5, 2021
@Martinsos Martinsos removed their assignment Mar 6, 2022
@Martinsos
Copy link
Member Author

There are couple approaches to consider here:

  1. We could somehow extend Operations in a way to provide access to headers in them -> basically give more control in them. Although there is probably a limit to what we can do here, we can't go completely custom, but maybe access to headers is already a big thing?
  2. We can give them access to wasp express app / routes in app.server.setupFn, so they can define their own routes there.
  3. We can provide a mechanism for injecting express middleware into server app in general.
  4. We can have special mechanism directly for defining custom HTTP routes.

4 is probalby the strongest solution, 2 is a quick way out, while 1 and 2 are probably useful in their own regard but don't address the problem directly / completely.

@Martinsos
Copy link
Member Author

Regarding middleware -> we talked a lot about it in some other capacities, e.g. Permissions, so I will also link to that #584 . @shayneczyzewski was big part of those previous conversations so also might have something to say here.

@Martinsos
Copy link
Member Author

Here is request from a Waspeteer that is related to this:

Trying to implement uploading files for my app. Has anyone tried doing this with wasp? Is there a good way to do this within wasp? Any good libraries?

Everything seems to go through multer, however I don’t see how we can add express middleware through wasp file or pass through headers to actions. Unless I am missing something?

@shayneczyzewski
Copy link
Sponsor Contributor

I think we all agree our main choices are escape hatch into Express (super easy, but lose lots of info), or we provide Wasp syntax.

The Wasp syntax could take many forms, but one quick one that may be nice is if we just provided them a way to say something like:

post { path: "/foo/<id:integer>", to: import { sayHi } from "@server/actions.js" }

So we would let them specify the HTTP verb as a keyword, and in the path we can also encode some type info. This may also dovetail into the top level data schema issue #642

@Martinsos
Copy link
Member Author

One more idea: What if we would allow them to define Operations with custom API?

Think about it as a hybrid between custom API routes and normal Operations -> Custom Operation.

So what Custom Operation let's you do is define its behaviour in more details, including what its REST API looks like. I guess you would implement a piece of logic on backend that handles a request and then returns args - that could be custom part. And then you would also have to implement a piece of logic on frontend that knows how to take args and produce an HTTP request. But then once we define those two pieces, the rest can again work automatically and have all the benefits of normal Operations, which is stuff like injecting entities via context, and certainly some other stuff in the future.

Oooo this sounds exciting to me! Not only do we get custom API but we get custom Operations! Maybe this turns out to be silly if we dig a bit deeper, or unfeasible, but it sounds like it could be potentially doable and pretty cool to me!

@shayneczyzewski
Copy link
Sponsor Contributor

One more idea: What if we would allow them to define Operations with custom API?

Think about it as a hybrid between custom API routes and normal Operations -> Custom Operation.

So what Custom Operation let's you do is define its behaviour in more details, including what its REST API looks like. I guess you would implement a piece of logic on backend that handles a request and then returns args - that could be custom part. And then you would also have to implement a piece of logic on frontend that knows how to take args and produce an HTTP request. But then once we define those two pieces, the rest can again work automatically and have all the benefits of normal Operations, which is stuff like injecting entities via context, and certainly some other stuff in the future.

Oooo this sounds exciting to me! Not only do we get custom API but we get custom Operations! Maybe this turns out to be silly if we dig a bit deeper, or unfeasible, but it sounds like it could be potentially doable and pretty cool to me!

Interesting idea. I wonder how much impact this would have on the TS stuff @sodic is doing, but this is definitely one way to go about it and it does feel a bit more aligned with our conceptual framework. One thing I wonder if this could support though is websockets? Maybe that is just another verb though, so users can define GET /blah/:id and also WS /socket? exe: https://www.npmjs.com/package/express-ws

@Martinsos
Copy link
Member Author

One more idea: What if we would allow them to define Operations with custom API?
Think about it as a hybrid between custom API routes and normal Operations -> Custom Operation.
So what Custom Operation let's you do is define its behaviour in more details, including what its REST API looks like. I guess you would implement a piece of logic on backend that handles a request and then returns args - that could be custom part. And then you would also have to implement a piece of logic on frontend that knows how to take args and produce an HTTP request. But then once we define those two pieces, the rest can again work automatically and have all the benefits of normal Operations, which is stuff like injecting entities via context, and certainly some other stuff in the future.
Oooo this sounds exciting to me! Not only do we get custom API but we get custom Operations! Maybe this turns out to be silly if we dig a bit deeper, or unfeasible, but it sounds like it could be potentially doable and pretty cool to me!

Interesting idea. I wonder how much impact this would have on the TS stuff @sodic is doing, but this is definitely one way to go about it and it does feel a bit more aligned with our conceptual framework. One thing I wonder if this could support though is websockets? Maybe that is just another verb though, so users can define GET /blah/:id and also WS /socket? exe: https://www.npmjs.com/package/express-ws

That is interesting! I was always imagining we will have a special abstraction for sockets, due to their "live" nature. I am having a bit hard time now imagining how this would work in the context of Operations, but I am sure it can in some way I just don't have enough knowledge at the moment. Interesting!

@Martinsos
Copy link
Member Author

Martinsos commented Jan 31, 2023

Related issue: #983 -> exposing app to setupFn. This is a very quick escape hatch that enables writing custom routes and that is also generally useful!

@shayneczyzewski
Copy link
Sponsor Contributor

I gave it a bit of thought @Martinsos, and tried to play off your idea about adding a route notion to Operations. What if it looked like something as simple as:

query getTask {
  fn: import { getTask } from "@server/queries.js",
  entities: [Task],
  route: GET "/task/:id(number)?show=:show(string)"  👈
}

The :name(type) segments in the route and query params implicitly become args to the query or action (so you don't even need a mapping function). You will also get req/res in the context when you use route.

You still invoke it in the same way:

const { data: task, isFetching, error } = useQuery(getTask, { id: taskId, show: "everything" })

In fact, we can now do a better job of warning the user if they do not supply everything. This now just becomes a URL like GET http://localhost:3001/task/123?show=everything instead of our default POST http://localhost:3001/operations/get-task.

You now get some extra things like this in your query, as mentioned:

export const getTask: GetTask<Task> = async ({ id, show }, context) => {
    // can use context.req or context.res
}

Users can then do things like context.res.send() as we can check for: https://expressjs.com/en/api.html#res.headersSent If they were sent, we do not do res.json(result) like we always do now.

Possible types:

  • string
  • number
  • bool

Maybe we can also define some notion of optionality with a ? prefix? Could be overkill though.

Possible verbs:

  • GET
  • POST
  • PUT
  • DELETE
  • ALL

Then, anything else more complex requires using startupFn(app) (e.g., web sockets)?

@Martinsos
Copy link
Member Author

Looks pretty cool @shayneczyzewski !

Ok, so basically we allow them to define custom HTTP route, with mapping from it to the args of the Operation.
We also expose req and res in context.

Are these two things orthogonal? We could expose req and res in context without introducing custom HTTP route, I think?
And we could introduce custom HTTP route without exposing req and res in context, right?

If so, should we always give them req and res? Or should we allow them to get access to them without specifying custom route?

By giving them control over req and res, can they mess something up?
I guess they could mess up req somehow by changing its values, but ok, that is really on them then.
Regarding res, right now we take returns value and return it as JSON. They can now return it in some other way, that is not a JSON -> that could be an issue? Or is it good? I am not sure how we can then deal with it if we want to use it on frontend and they don't return JSON. But ok that is their problem then I guess?

How much flexibility can they achieve with this approach you suggested?
I guess they have max flexibility? They can declare any kind of route really, and then they can just parse req and do anything with res. Can they not specify any names/args in route and then manually extract them from req, form query params?

Instead of us being smart and implementing this small language/DSL for routes that maps from route to args and vice versa, we could just let them provide two functions: one that receives route as a string and produces JS object, and vice versa. So basically they could define those mappings on their own, via two functions. But I do like the idea that we do this for them, that is quite nice indeed, just wanted to think through this.

We could add json to types.

Yup, we should also have startupFn(app) as final escape hatch.

@Martinsos
Copy link
Member Author

Martinsos commented Feb 13, 2023

Had a quick chat with @shayneczyzewski , we concluded that we could think about this in following steps:

  1. We can implement escape hatch via startupFn(app) as very first step. This should already enable any kind of custom api routes, but is a bit manual / ugly.
  2. We can make Operations more customizable by providing users a way to customize URL/route with stuff like route: GET "/task/:id(number)?show=:show(string)". Also, offering them access to req, maybe -> not sure if that is needed. We concluded that we don't want to give them access to res, because they can just mess things up (what kind of Operation is it if it doesn't return JSON? What would be weird). So maybe req, not res.
    But the thing is, this is not really custom routes then, it doesn't solve that use case, it is instead customizing Operations. How needed is this customization of Operations though? Not sure.
  3. As a continuation on step 1, we might watn to provide nicer way to customize routes than just that setupFn escape hatch. We could do this with interface similar to Operations -> smth like api myTwitterCallback { fn, entities, httpRoute } -> actually what @shayneczyzewski described here Allow defining custom API routes (http) #268 (comment) . But it would not work as an Operation, you couldn't call it on frontend with useQuery, and it would be less restricted -> it would give you both req and res, you can return what you want, ... . So it would have different guarantees / expectations.

It is good to remind ourselves that main use case for custom routes is for callbacks and some custom stuff, not for RPC between client and frontend -> for RPC we have operations.
Operations = RPC, custom routes = anything.

I think we certainly need to do (1) and I do think we should also go for (3) after it, it is much nicer than (1).

I think (2) is also attractive, but we should evaluate a bit how useful it is, and we should check with @sodic how it plays with adding types to Operations, because there will be some interplay there almost certainly.

@sodic
Copy link
Contributor

sodic commented Feb 15, 2023

@Martinsos @shayneczyzewski I'll reply to the conclusions outlined in the last comment.

  1. We can implement escape hatch via startupFn(app) as very first step. This should already enable any kind of custom api routes, but is a bit manual / ugly.

I agree this makes sense as the first step.

  1. We can make Operations more customizable by providing users a way to customize URL/route with stuff like route: GET "/task/:id(number)?show=:show(string)". Also, offering them access to req, maybe -> not sure if that is needed. We concluded that we don't want to give them access to res, because they can just mess things up (what kind of Operation is it if it doesn't return JSON? What would be weird). So maybe req, not res.

Can't say I agree with this one. I think we should either:

  • Replicate Express's usual interface (i.e., go low level and expose everything they expose, both res and req).
  • Come up with our own higher-level API.

Exposing req without res feels weird to me.

But the thing is, this is not really custom routes then, it doesn't solve that use case, it is instead customizing Operations. How needed is this customization of Operations though? Not sure.

Me neither 😄. One more reason to stick with the "dumb" serverSetup approach for now.

  1. As a continuation on step 1, we might want to provide nicer way to customize routes than just that setupFn escape hatch. We could do this with interface similar to Operations -> smth like api myTwitterCallback { fn, entities, httpRoute } -> actually what @shayneczyzewski described here Allow defining custom API routes (http) #268 (comment) . But it would not work as an Operation, you couldn't call it on frontend with useQuery, and it would be less restricted -> it would give you both req and res, you can return what you want, ... . So it would have different guarantees / expectations.

This makes a lot of sense to me. I even like that you can't use it with useQuery. That should hopefully prevent users from abusing the api for everything

It is good to remind ourselves that main use case for custom routes is for callbacks and some custom stuff, not for RPC between client and frontend -> for RPC we have operations. Operations = RPC, custom routes = anything.

I think we certainly need to do (1) and I do think we should also go for (3) after it, it is much nicer than (1).

Yes, Exactly!

I think (2) is also attractive, but we should evaluate a bit how useful it is, and we should check with @sodic how it plays with adding types to Operations, because there will be some interplay there almost certainly.

Types shouldn't be an issue here, but I'd definitely punt on (2) for now.

@shayneczyzewski
Copy link
Sponsor Contributor

Thanks @Martinsos and @sodic. I appreciate the discussion and thoughts. I agree with the next steps. So the plan will be:

  • Implement startupFn(app) first; I will do this next
  • Do not do any route customization of Operations at this time
  • Start on api myTwitterCallback { fn, entities, httpRoute } when needed (possibly after step 1, but possibly after more important items this release, we can decide)

Thanks again! 🚀

@Martinsos
Copy link
Member Author

Martinsos commented Feb 16, 2023

@shayneczyzewski sounds like a plan!

@sodic although we agree about punting on (2) for now, I wanted to follow up on couple things, for the sake of further discussion we might do on this topic:

  1. Why would it be weird giving only req and not res? You said you have weird feeling about it, which is valid, usually feeling is coming from somewhere, but I don't have weird feeling in this case, so I would love to understand some arguments if you can unpack it a bit.
    Btw term I saw being used for stuff like this is that it "smells" :) -> e.g. "this looks smelly to me". Also term "code smell" is often used.
  2. We could maybe wrap req a bit into something, if we want to provide our own higher API I guess, although I am not sure if we want to guess right now what they need.
  3. You said types shouldn't be an issue. What I was interested in is if we do the route thing where we describe the types in it "(int), (bool)", how would that play with what you are working on, which is typing the Operation payloads? Aren't we then repeating the type information?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants