oak icon indicating copy to clipboard operation
oak copied to clipboard

Type inference fails for routes with escaped colons

Open bodograumann opened this issue 4 years ago • 2 comments

When a route has the form "/some/path/:param\\:action", the run time parameter is called param, but the type inference generates { "param\\:action": string }.

The only way to workaround it, is to use

router.post<
  "/some/path/:param\\:action",
  { param: string; "param\\:action": string },
  AppState
>(
  "/some/path/:param\\:action",
  …
);

I much preferred the way it was before type-inference. The reusable middlewares have so much ugly boilerplate now:

async function checkWriteAccess<
  R extends string,
  P extends { param: string }
>(context: RouterContext<R, P, AppState>, next: () => Promise<unknown>) {
  …
}

bodograumann avatar Nov 18 '21 09:11 bodograumann

I much preferred the way it was before type-inference.

I strongly disagree. I can see the problem with more complex paths, but I would argue that these are more of an exception. Most users' paths will be fairly simple, and type inference for these use cases is a great quality-of-life feature (and one of the features that blew me away when I started using oak).

Imho, it's acceptable to have to write a little boilerplate code for more complex paths in exchange for not having to write any extra code most of the time and still enjoying type-safe routes 🙃

DerZade avatar Jan 10 '22 13:01 DerZade

Ok, I agree the type-inference is quite helpful in the most common use case.

What I am most concerned about, is that the inferred typing in my case is completely wrong. One big downside of the current way that url templates are written is, that we only define the start, but not the end of path parameters. That is in line with how express does it, but using {param} would exclude many more of these edge-cases. Currently you can only use /, . and - after a parameter and still get correct type inferrence. This is not documented anywhere afaict.

One idea would be to refactor the route matching from the router and give two options. The current matcher and a matcher where parameters are written as {param}. Might be a bit overkill though :-D

As long as the route name can contain arbitrary complex regular expression, it will be unavoidable that the naïvely inferred parameter types do not match the actually produced types sometimes. In these cases it should be easier to override the parameters. The user should not

  • have to know about implementation details,
  • need to include the url, when the parameters cannot be inferred from it anyway nor
  • need to include the wronly inferred parameters.

I now realized, that this is actually doable as:

router.post<string, { param: string }>("/some/path/:param\\:action", …);

So there are two things I suggest:

  1. Add another signature, that allows leaving away the first generic argument, so that we can write directly router.post<{ param: string }>("/some/path/:param\\:action", …);. The implementation already has this signature, but in typescript “The signature of the implementation is not visible from the outside.”.
  2. Document this way of overriding the inferred path parameter types in the readme, so nobody else has to trudge through the code for hours, before realizing how easy it could have been.

bodograumann avatar Apr 16 '22 09:04 bodograumann