Conditionally enrich context and change it's type for all subsequent midleware.
mordv opened this issue · 7 comments
By far my understanding is that, during middleware chain, Context type is only changed through filters and all middleware must explicitly flavor Context type globally.
But what if I have a middleware which usage only makes sense after some filtering was done and it also modifies the context, extending it's type?
export const enrichContextWithTargetUser = (ctx: Filter<ChatTypeContext<Context, 'supergroup'>, 'message:is_topic_message'>, next: NextFunction) => {
ctx.targetUser = AppStorage.getUserForThreadId(ctx.message.message_thread_id!);
// ???
}
bot.chatType('supergroup').on('message:is_topic_message').use(enrichContextWithTargetUser).use(async ctx => {
ctx.targetUser; // this should be typed properly
}Some related example from trpc library, where middleware modifies ctx type for all subsequent functions. There, context isn't mutated directly but passed to the next function instead.
But what if I have a middleware which usage only makes sense after some filtering was done and it also modifies the context, extending it's type?
Then this filtering can narrow down the type to a custom, more narrow type by using a type predicate with bot.filter.
Since there are multiple bot.use statements that mutate the bot object's type, we cannot do this any other way. The only solution would be to move away from mutating statements entirely, and use a purely declarative syntax for middleware, but this is much harder to use, looks less familiar, and will generally alienate away newcomers.
People have suggested less explicit ways of changing the bot object, but that's just type casts in disguise so I won't ship that stuff.
Then this filtering can narrow down the type to a custom, more narrow type
Yes I can narrow down middleware input context. But how to properly type context modified by middleware?
Since there are multiple bot.use statements that mutate the bot object's type
Honestly, I didn't understand what do you mean. For me the solution is the matter of adding second type param to Middleware, adding types to NextFunction and proper typings for use:
use<T extends C>(...middleware: Array<Middleware<C, T>>): Composer<T> {
const composer = new Composer(...middleware);
this.handler = concat(this.handler, flatten(composer));
return composer;
}But that's may be naive.
Then this filtering can narrow down the type to a custom, more narrow type
Yes I can narrow down middleware input context. But how to properly type context modified by middleware?
Using a type predicate. Look:
const t: Composer<T> = composer.filter((ctx): ctx is T => {
ctx.prop = value;
return true;
})In TypeScript, type predicates always are implicit type casts.
Since there are multiple bot.use statements that mutate the bot object's type
Honestly, I didn't understand what do you mean. For me the solution is the matter of adding second type param to Middleware, adding types to NextFunction and proper typings for use:
use<T extends C>(...middleware: Array<Middleware<C, T>>): Composer<T> { const composer = new Composer(...middleware); this.handler = concat(this.handler, flatten(composer)); return composer; }
This code does not type-check. You will have to add an explicit type cast to the implementation.
But that's may be naive.
It is not. This has been discussed before, and we were considering to introduce bot.cast<T> that simply casts the context type. However, type casting is unsafe, so we should ideally make it as explicit as possible. Adding a new method to hide away the type cast (or---even worse---secretly performing type casts internally in bot.use) increases the chance of bugs, so it simply makes bot code worse.
If you want to perform type casts, simply do them explicitly:
const narrow = (composer as Composer<T>).use(async (ctx, next) => {
ctx.prop = value;
await next();
});Using a type predicate. Look:
Mutating inside filter is by any means ugly, I think you'd agree. But at least thanks for a workaround.
This code does not type-check. You will have to add an explicit type cast to the implementation.
I don't think so. My assumption is that we can add second type to middleware and type NextFunction properly. Again, have you checked the link I provided? Can it possibly be implemented the way they did?
or---even worse---secretly performing type casts internally in bot.use
Why secretly? It's inferred, yes, but without dirty tricks.
Using a type predicate. Look:
Mutating inside filter is by any means ugly, I think you'd agree. But at least thanks for a workaround.
I do agree, but you were explicitly asking for filtering and type casting in one, so if there were any meaningful filtering condition in the code, it would look muss less like abuse.
This code does not type-check. You will have to add an explicit type cast to the implementation.
I don't think so. My assumption is that we can add second type to middleware and type NextFunction properly. Again, have you checked the link I provided? Can it possibly be implemented the way they did?
I have, and we have played around with similar implementations, but the code rapidly gets messy with fully declarative middleware systems. If you have 50 handlers installed after 10 plugins and 5 custom pieces of middleware, you'll end up with deep nesting or lots of helper variables. If you can draft an implementation for a middleware system that is better, please share it here, but especially @wojpawlik tried many options and so far there has not been a convincing result.
or---even worse---secretly performing type casts internally in bot.use
Why secretly? It's inferred, yes, but without dirty tricks.
Casting with as would happen at some point inside the library and the developer would not know this. It is better to cast explicitly.
grammY is already pushing TypeScript to its limits.
grammY's Composer could allow sub-trees with independent ctx, but we couldn't think of any practical use-case.
The very foundation of middleware trees prevents us from doing use(...): asserts this is Composer<???>:
Lines 225 to 226 in 4280050
Closing due to inactivity. Please feel free to re-open if you still have more things to share.