Mediator icon indicating copy to clipboard operation
Mediator copied to clipboard

#103 - Add support for ICommand IRequest without Unit

Open mvput opened this issue 2 months ago • 10 comments

Implementation for #103

  • Added void request and command messages for a clean separation
  • Pipeline and handlers without Unit type
  • Send implementation without response
  • Added some unit tests and updated some unit tests
  • Updated mem alloc test to response type

mvput avatar Sep 30 '25 19:09 mvput

Thanks for the PR! Will have a look this weekend

martinothamar avatar Oct 01 '25 20:10 martinothamar

Hmm I did not realize we would end up having completely different pipelines (I would not have asked the pipeline ordering question if I had realized that). This feels like pretty bad UX if we consider that users who implement pipeline behaviors and have responseless messages would have to essentially duplicate their behaviors.. Aren't we essentially trading return default (for the unit) for duplication of pipeline behaviors in that case?

We would close the gap on the drift from the MediatR contract with this but open up this kind of unexpected behavior difference in terms of pipeline behaviors. I'm starting to think the only viable options here are to do the same thing as MediatR does or not do it at all. Thoughts?

martinothamar avatar Oct 05 '25 20:10 martinothamar

True, keeping in mind closing the gap with MediatR i agree in having a new unexpected behavior with the pipelines.

I will adjust this to reuse the same pipelines.

mvput avatar Oct 06 '25 06:10 mvput

@martinothamar updated the PR. Created a Void wrapper that returns the default unit. Let me know if this implementation works for you!

mvput avatar Oct 06 '25 09:10 mvput

Thanks for the feedback and comments. Just to update I'm busy with rewriting and improving the code and implementation.

mvput avatar Oct 28 '25 20:10 mvput

@martinothamar I finished the rework. Before finalizing the PR and tests I want to double check if the approach works.

  • The async/await is removed. To drop the Unit value, it uses casting to ValueTask. Also for the for returning the Unit value I created a ValueTaskSource implementation which returns a default Unit value. First the task is check if completed otherwise wrap it.
  • The collections remain separated for unit and response requests
  • Most logic is now moved the models, only a few small branches remain in the template file. Renamed it to HasResponse and created a Request/Command wrapper type with and without response.
  • Implemented Singleton in the UnitWrapper classes
  • Removed the interface for the UnitWrapper and directly call it
  • Updated unit test with a Unit Request allocation , which remains zero now (which is nice)

If this all works out I will check the remaining issues

mvput avatar Oct 29 '25 19:10 mvput

@mvput would be great to have this available, but looks like there is a conflict blocking from eventual merge? 👀 @martinothamar

DevTKSS avatar Nov 19 '25 15:11 DevTKSS

@mvput would be great to have this available, but looks like there is a conflict blocking from eventual merge? 👀

Yes, currently awaiting the feedback from @martinothamar. If all works out I can finish the work for this PR.

mvput avatar Nov 19 '25 15:11 mvput

Hey, sorry, been busy trying to wrap up work for 3.1.. This PR will go into v4 (breaking changes). We need to give 3.1 some time to stabilize at it is currently in preview. Doing a quick pass through this now

martinothamar avatar Nov 19 '25 21:11 martinothamar

Good day everyone.

I've this idea and it's killing me. I've share it and my thought would give me some peace. Maybe, it would be better to add support for IRequest without Unit in v3.2? Also, let's just add analyzer warning to remove the Unit while keeping both versions for while, up until v4.0 release? And completely delete the Unit in that release?

I mean, people are using this Package and it's critical change. Wouldn't it be better to give everyone some time to just adapt to the reality?

what do you think?

KarimovJavoxir avatar Dec 09 '25 19:12 KarimovJavoxir