curveball / core

The Curveball framework is a TypeScript framework for node.js with support for modern HTTP features.
https://curveballjs.org/
MIT License
526 stars 7 forks source link

About the type of ctx.state #129

Closed alinnert closed 2 years ago

alinnert commented 4 years ago

First of all, I found this framework because your blog post showed up in Google News Feed on my phone. The timing is really incredible. I'm planning a Node-App right now and I recently reached the point where I really want to use TypeScript, which I found out isn't by far as easy and nice for Node as I imagined. The biggest pain-point was how frameworks move data from a middleware to another or the request handlers. I even started creating my own frameworks in my head and wonder how this could be solved in a type safe way.

Now, I'm happy that in Curveball at least an index signature is used for ctx.state, but I think there's still room for improvement. I've created an issue on Fastify how they could improve on it. Unfortunately, they won't change the API for TypeScript.

What I basically would love to see is: If a (auth) middlware fetches the current user and connects it with the current request, and later I want to access this user I want to get a value of type User, not any or unknown (or an error because the property doesn't exist, which happens with most frameworks so far).

Are you planning to improve this behavior? What do you think about the ideas I posted in the other issue? Or are there better alternatives?

There are just so many frameworks out there that are written in a strongly typed language (C#, Java, Go, ...). They also have to solve this somehow without using any if the language doesn't support it.

evert commented 4 years ago

I have one specific idea for this, let me know if this makes sense for your case.

Instead of putting your information in ctx.state, make a new property: ctx.auth.

It's possible to type this by extending the interface:

declare module "@curveball/core" {
   interface Context {
     auth?: YourAuthType
   }
}

This fully types your property, and you can make it optional (if desired) forcing users of this property to ensure that a middleware was called earlier to set it.

Alternatively I was thinking of adding a TState generic as the 3rd generic parameter, but I worry that that's not nearly as ergonomic as extending the interface, especially if a few different middlewares compose ctx.state.

alinnert commented 4 years ago

This advice also exists for Fastify. I don't like it for a few reasons. The extended interface is global while the real type of the state changes over time. This also means that every state property will be undefined at first. You also have to manage and update the types by hand that get added by 3rd party middlewares.

The thing is, if you use middleware A, then B, B has access to properties set by A. The opposite is not the case though. But the interface suggests otherwise. Therefore the current typing is actually more correct.

If you use a 3rd type parameter for the state one could indeed merge the state's type by combining the types that middleware packages need to export. This way the user don't have to manually keep individual property types up-to-date. But the order is still incorrect.

The optimal way would be if the state's type changes automatically over time, using type information the middleware provides to the framework. The other idea I still have in mind is to move the state out of the context, into the middlware package (like my solution 3 in the link above). Wouldn't that be feasible?

Here's an example API I have in mind:

authMiddleware.ts

import { State } from '@curveball/core'

interface User { }

export const currentUser = new State<User>() // [1]

function authMiddleware(ctx) {
  const user = getUser()
  currentUser.set(ctx, user) // [2]
}

Some handler or another middleware:

import { currentUser } from 'auth-middleware'

function handlerOrMiddleware (ctx) {
  const user = currentUser.get(ctx) // [1]
}

Also, since state values work with handles instead of properties they can't be overridden (by accident or maliciously) that easily. And I think it's not much more code than using ctx.state.

At least this works in my head. What do you think about this?

evert commented 2 years ago

This is something I think I looked into a few times, but haven't found a satisfying way to implement this =(