#103 - Add support for ICommand IRequest without Unit #228

mvput · 2025-09-30T19:05:12Z

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

martinothamar · 2025-10-01T20:35:36Z

Thanks for the PR! Will have a look this weekend

martinothamar · 2025-10-05T20:22:08Z

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?

mvput · 2025-10-06T06:00:03Z

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 · 2025-10-06T09:56:28Z

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

martinothamar

Thanks for working on this! Looking promising, left some comments. If you feel stronly opposed to any of them feel free to say so 😄 Additional comments:

We should add responseless requests/commands to common/Messages.cs, that will let us catch som additional bugs as we know have more logic/branching related to this
I think it makes sense to introduce some benchmarking to compare the performance of sending requests with and without responses, since we now likely will incurr some overhead for messages without responses due to having to manage the transitions between having Unit and not. These benchmarks don't have to compare against other libraries, I think just having 2 methods comparing with and withour response is good enough

samples/basic/Console/Program.cs

src/Mediator.SourceGenerator/Implementation/Analysis/CompilationAnalyzer.cs

src/Mediator.SourceGenerator/Implementation/Models/RequestMessageHandlerWrapperModel.cs

src/Mediator.SourceGenerator/Implementation/Models/RequestMessageModel.cs

src/Mediator.SourceGenerator/Implementation/resources/Mediator.sbn-cs

test/Mediator.MemAllocationTests/SomeRequestMemAllocTracking.cs

....Tests/_snapshots/BasicTests.Multiple_Notification_Handlers_One_Class#Mediator.g.verified.cs

martinothamar · 2025-10-25T08:56:48Z

test/Mediator.Tests/Pipeline/PipelineTests.cs

+
+        var original = pipelineSteps.Select(p => p.LastMsgTimestamp).ToArray();
+        var ordered = pipelineSteps.Select(p => p.LastMsgTimestamp).OrderBy(x => x).ToArray();
+        Assert.True(original.SequenceEqual(ordered));


Isn't there only 1 pipelinebehavior here? I think we need to add more for this test to add value

src/Mediator.SourceGenerator/Implementation/resources/Mediator.sbn-cs

mvput · 2025-10-28T20:34:57Z

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

mvput · 2025-10-29T19:15:09Z

@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

DevTKSS · 2025-11-19T15:25:31Z

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

mvput · 2025-11-19T15:31:22Z

@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.

martinothamar · 2025-11-19T21:46:02Z

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 · 2025-11-19T21:49:00Z

src/Mediator/Pipeline/ValueTaskWrapper.cs

+
+namespace Mediator;
+
+public class ValueTaskWrapper<T> : IValueTaskSource<T>


Does this actually save an allocation? If we need to construct this for the async path anyway. I think it would be useful to have some benchmarks results here comparing the performance of

sync handler, before and after this PR

async handler, before and after this PR

For requests with unit/without response

Once we have validated the design, if we end up sticking with this, it should just stay in the generated code as an internal type (internals namespace). I don't want this to be part of the public contract

Will run some performance benchmarks later this weekend!

Performed the benchmarks on 3.0 and 4.0-preview (this PR). The results are attached in the comment.
Both runs are done in the same conditions on the workstation. Will do some analysis on the results
benchmark_v4.0-preview.txt
benchmark_v3.0.txt

Based on the performance benchmarks I did some improvements with the task/unit handling. The updated results show a better result (with a small bytes allocation)
benchmark_v4.0-preview2.txt

KarimovJavoxir · 2025-12-09T19:01:41Z

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?

mvput added 5 commits September 25, 2025 22:49

Add void type

bf53219

Add void check

303fa7b

Fix type

0d545a4

Add tests and finalize

f463ea8

Merge branch 'main' into feature/martinothamargh-103-v2

b5baf8f

mvput marked this pull request as ready for review September 30, 2025 19:17

Add void wrapper

f536d3b

mvput and others added 2 commits October 6, 2025 12:01

Update Program.cs

510d382

tiny typo to fix scoped runs

8cc279b

martinothamar requested changes Oct 25, 2025

View reviewed changes

mvput added 4 commits October 26, 2025 10:07

Remove local test

c573ca5

Improve branching strategy

5ecaeb2

Fix void issue

7aa4557

Add unit response handlers

0afe5f5

mvput added 5 commits October 29, 2025 17:30

Implement value task

996578c

Update unit implementation

8421dc3

Fix alloc issue with tasks

0ae806e

Fix many command/request issues

c4dae84

Rerun tests

096eeed

martinothamar reviewed Nov 19, 2025

View reviewed changes

Add unit/async benchmarks

749b8ac

Use wrapper

63078ef


		namespace Mediator;

		public class ValueTaskWrapper<T> : IValueTaskSource<T>

#103 - Add support for ICommand IRequest without Unit #228

Are you sure you want to change the base?

#103 - Add support for ICommand IRequest without Unit #228

Uh oh!

Conversation

mvput commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martinothamar commented Oct 1, 2025

Uh oh!

martinothamar commented Oct 5, 2025

Uh oh!

mvput commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mvput commented Oct 6, 2025

Uh oh!

martinothamar left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

martinothamar Oct 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mvput commented Oct 28, 2025

Uh oh!

mvput commented Oct 29, 2025

Uh oh!

DevTKSS commented Nov 19, 2025

Uh oh!

mvput commented Nov 19, 2025

Uh oh!

martinothamar commented Nov 19, 2025

Uh oh!

martinothamar Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

martinothamar Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

mvput Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

mvput Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvput Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KarimovJavoxir commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mvput commented Sep 30, 2025 •

edited

Loading

mvput commented Oct 6, 2025 •

edited

Loading

martinothamar left a comment •

edited

Loading

mvput Dec 2, 2025 •

edited

Loading

mvput Dec 3, 2025 •

edited

Loading

KarimovJavoxir commented Dec 9, 2025 •

edited

Loading