Update IRequestHandler to return ValueTask instead of ValueTask<Unit>
I assume that this type was added for compatibility with MediatR. However, it is removed in latest MediatR versions - which is a good thing in my opinion, as it is much cleaner to write Task return type instead of Task<Unit>.
Does it make sense to remove it in the next major version?
Probably, I'll look into it as part of 3.0. It makes sense to keep up with MediatR API as much as possible
Any update on this? IT's one of the (small) obstacles that we are currently facing migrating from MediatR
Any update on this? IT's one of the (small) obstacles that we are currently facing migrating from MediatR
I think if it's so important to migrate then u can use an adapter pattern
Im currently using the following interface as a workaround, not ideal though
public interface IRequestHandler<in TRequest> : IRequestHandler<TRequest, Unit>
where TRequest : IRequest<Unit>
{
new ValueTask Handle(TRequest request, CancellationToken cancellationToken);
async ValueTask<Unit> IRequestHandler<TRequest, Unit>.Handle(TRequest request, CancellationToken cancellationToken)
{
await Handle(request, cancellationToken);
return Unit.Value;
}
}
This is also our last (small) obstacle to move over from mediatR with our projects. That being said is there anything we can do to move this along? Maybe someone can point me in the correct direction to develop/open a pr for this?
I haven't looked into this much yet, but I'm guessing it's going to be hard to avoid dropping that Unit somewhere since the pipeline behaviors will still have to keep TResponse. Here is a rough outline of what needs to be done/investigated:
- Update
IRequestHandlerinterface - Update
CompilationAnalyzerto find the new flavor of requesthandler (there are probably places where we expect 2 type parameters) - Source generate new
ValueTask Send<TRequest>(TRequest request, ...);overloads - Add message handler wrappers that have no
TResponseto matchIRequestHandler(find a cheap way to dropUnitcoming from pipeline)
Does anyone here want to work on this?
I would like to try and implement this. There are 2 caveats to this however. I'll be going on vacation this Friday and will be offline for at least 2 weeks. I could start somewhere after that. I'm also unfamiliar with the project and haven't done much with source code generators before but it seems like a nice challenge.
If there is someone else who could pick this up sooner or has more experience I won't be demoralized/ impacted if you give it away at any point in the process :).
I'm available to pick this up and work this out
@martinothamar yesterday I was working on this feature and I want to know your take on the following.
I think the most correct way to implement this is to completly ditch the TResponse for the source generation.
ISenderwill have a methodSendwhich returns aValueTaskIRequestandICommandwithout response type, same forIRequestHandlerandICommandHandler
The pipeline behavior without a TResponse generic constraint
public interface IPipelineBehavior <TMessage> where TMessage: notnull, IMessage
{
ValueTask Handle(TMessage message, MessageHandlerDelegate <TMessage> next, CancellationToken cancellationToken);
}
For example you have a validation pipeline you can either implement both or either one of them.
public class ValidationPipelineBehavior: IPipelineBehavior <TRequest> ,
IPipelineBehavior <TRequest, TResponse>
{
ValueTask Handle(TMessage message, MessageHandlerDelegate <TMessage> next, CancellationToken cancellationToken);
ValueTask<TResponse> Handle(TMessage message, MessageHandlerDelegate <TMessage, TResponse> next, CancellationToken cancellationToken);
}
Do you mean adding new overloads to ISender etc? I think that is what MediatR does, except that there is only 1 method in the pipeline behavior and then the "handler wrapper" for responseless messages/handlers just drops the unit. I'm fine with that approach I think
@martinothamar correct MediatR wraps the handler en ditches the unit. The other option (which i slightly prefer) is overload ISender and also create a pipeline behavior without unit, so no need for wrapping the handler and dropping unit.
How will we determine the order of behaviors?
options.PipelineBehaviors = [typeof(BehaviorWithResponse0), typeof(BehaviorWithoutResponse0), typeof(BehaviorWithResponse1)];
Or the DI equivalent. Unless I'm misremembering completely we just get a list of services from DI and the order is correct due to order of registration or config
I think the order of either the behavior with response or without, so the order remains the same as currently.
A small update, got everything working. Now testing and cleaning up the code. Hopefully ready for PR later this week.
I've been also waiting for this as well! The package is already really useful! But, it would be even better! And I believe that it would attract more perfectionist developers like myself!