Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.0 tracking issue - rewriting source generation #98

Open
4 of 9 tasks
martinothamar opened this issue Jun 21, 2023 · 36 comments
Open
4 of 9 tasks

3.0 tracking issue - rewriting source generation #98

martinothamar opened this issue Jun 21, 2023 · 36 comments
Milestone

Comments

@martinothamar
Copy link
Owner

martinothamar commented Jun 21, 2023

Things I want to solve/merge in 3.0.

To solve these I'll be rewriting the source generation process. I will be starting out in an experimental branch
and seek feedback before bringing it into main (which is targeting 3.0 preview).

The task list is sorted from most difficult to least difficult (assumed).

@martinothamar martinothamar added this to the 3.0 milestone Jun 21, 2023
@martinothamar
Copy link
Owner Author

Another thing I've considered: should this library be renamed? "Mediator" is kind of plain/vanilla. It's basically a working title that I never changed. Feel free to drop suggestions and feedback..

@TimothyMakkison
Copy link
Contributor

Improve performance for large number of messages #440

Have you looked at using liquid instead of scriban? Fluid appears to be more performant than scriban. Not sure if liquid supports all your requirements

@martinothamar
Copy link
Owner Author

Not considered it yet, but open to it! Would be fairly easy to migrate I think. I'd like to hold off on that until the top 4 issues are completed though, since there will be significant changes in the source generator

@TimothyMakkison
Copy link
Contributor

TimothyMakkison commented Jun 27, 2023

I'm not sure how much scriban affects performance tbh. Perhaps parsing the template once and reusing it could help, although I'm not sure if source generators allow this.

I'd like to hold off on that until the top 4 issues are completed though, since there will be significant changes in the source generator

Looking forward to seeing your changes, this project has been super helpful when wokring on Mapperly.

@ejsmith
Copy link

ejsmith commented Jun 30, 2023

This project looks awesome! I'm interested in a conventional handler approach either on top of this library or as an additional source generator. Something that decouples the handlers from the messaging library like this: https://wolverine.netlify.app/guide/handlers/ It would just discover handlers by convention and generate the concrete handler that implements your IRequestHandler and friends interfaces. Any interest in this?

@martinothamar
Copy link
Owner Author

So that would only affect handlers right? The requests/notifications would still use the appropriate interface?

Related: one thing I like a lot about Wolverine (and Marten) is that there is support for static functions as handlers. The typical case for request/notification handlers is that they are stateless, so it feels silly allocating an object simply to invoke a method. There are often injected services, but they can be injected as parameters to the function as well.. Seeing as this is a performance oriented library, that's something I would like to explore in the future. ASP.NET Core minimal APIs is kind of also going in that direction, having static functions as handlers, and we can now have static abstract methods in interfaces.. Might be something there.

So yes there is some interest there from my side for sure (taking inspiration from the Wolverine approach in general), but I'm not sure how it could be integrated into this library. I'll think some more about this while refactoring the source generator.

Btw, a challenge with source generators is that they don't compose at all, so I don't think this is solvable as an additional source generator (the Mediator source generator wouldn't have access to code generated by another source generator)

@ejsmith
Copy link

ejsmith commented Jul 1, 2023

Yes, this would only affect the handlers and not sending the messages.

Yeah, it really sucks that source generators can't simply be ordered by a priority with same priority using the same compilation model, but any subsequent ones would get an updated compilation model and be able to inspect code from previous priorities.

@ejsmith
Copy link

ejsmith commented Jul 7, 2023

Was just looking at dotnet/aspnetcore#48817 and thinking how doing something similar would make an amazing mediator implementation.

@martinothamar
Copy link
Owner Author

Yea it's on our radar for sure. @Tornhoof also brought it up here: #7 (comment)

Since they are bringing it into ASP.NET Core atm I'm assuming the interceptors proposal is basically approved for .NET 8/C# 12? 😄
I am very tempted to go all in on .NET 8 SDK requirement and using interceptors. I haven't been able to come up with a design for 3.0 that solves all the issues in the OP (while keeping the perf that is), but with interceptors everything becomes a lot simpler

@ejsmith
Copy link

ejsmith commented Jul 8, 2023

Yeah, interceptors was merged into main.

@TimothyMakkison
Copy link
Contributor

TimothyMakkison commented Jul 8, 2023

It might be better to wait for these changes, but will you be adding support for incremental pipeline caching? The output from CompilationAnalyzer could be turned into an Equatable friendly type.

I'd be willing to have a go at this.

@KennethHoff
Copy link

Since they are bringing it into ASP.NET Core atm I'm assuming the interceptors proposal is basically approved for .NET 8/C# 12? 😄

Most likely, but one thing I've realized about the dotnet codebases is that they'll update everything the moment it's available to be done. It's really quite interesting. They rewrote practically every instance they did null checks for parameters with the !! proposal a few years back before that was reverted. Same with ArgumentNullException.ThrowIfNull(param), and they've said they will with primary Constructors whenever that spec gets finalized.

So while it most likely will pass, I wouldn't go all-in just yet, especially since they've repeatedly said that it is a preview feature that likely will get changed or maybe even removed (although I highly doubt the latter due to just how important it is for the AoT story they're going all-in on)

@ejsmith
Copy link

ejsmith commented Jul 13, 2023

They are somewhat committed to it because of the AOT story and they are going to be using it a lot (already has multiple PRs accepted) to generate code that supports that. They would need to rewrite a lot of their code as well. Not to mention that this will be a pretty straight forward thing to replace if they come up with a different similar approach.

@TimothyMakkison
Copy link
Contributor

TimothyMakkison commented Sep 20, 2023

I am very tempted to go all in on .NET 8 SDK requirement and using interceptors.

#7 (comment)

It's already possible to create a pseudo-interceptor in all versions of .NET. By using the caller line number and caller file path, it is possible to differentiate each caller and respond appropiately.

public static void AddMediator(.... parameters go here, [CallerLineNumber] line = 0, [CallerFilePath] file = "")
{
    // an efficient condition can be built using the line number, file path length and varying chars in the file path
    // not sure how much the compiler will do
    return (line, path) switch
    {
        (0, _) => Mediator1.AddMediator(services),
        (42, { Length: 15 }) => Mediator2.AddMediator(services),
        (42, { Length: 21 }) => Mediator3.AddMediator(services),
        _ => throw new ArgumentOutOfRangeException()
    }
}

public static TResponse Send<TRequest, TResponse>(.... parameters go here, [CallerLineNumber] line = 0, [CallerFilePath] file = "")
{
    ...
}

Note that this may not work if the same method is called repeatedly on the same line. This is very unlikely to occur for Send or AddMediator; however, a check and diagnostic could be used to prevent this.

@dev-anton-ko
Copy link

Speaking about "Support generic messages".
Is this including support of generic Queries/Requests/Commands?

@martinothamar
Copy link
Owner Author

Yes, thats not possible currently, and requires rewriting the source generation of the IMediator implementation

@KANekT
Copy link

KANekT commented Nov 22, 2023

@martinothamar Project is frozen or is there a plan and date for the new version ?

@martinothamar
Copy link
Owner Author

Not fozen, but there is no date/deadline either. How much time I get on this depend on dayjob, personal life etc. Next steps for me is going over and merging the great stuff @TimothyMakkison has been doing, then see what version 3.0 actually becomes

@KANekT
Copy link

KANekT commented Nov 22, 2023

ok, Thanks. I'm currently using version 3 in my project. Therefore, I am interested in the further fate of the package.

@JohnGalt1717
Copy link

My personal request is for integrations with Redis/RabbitMQ/Kafka etc. to incrementally take this into doing distributed requests and requests/responses.

I'd love to have a library that does mediator first and then transparently goes up the chain to distributed requests automatically.

@KANekT
Copy link

KANekT commented Dec 22, 2023

My personal request is for integrations with Redis/RabbitMQ/Kafka etc. to incrementally take this into doing distributed requests and requests/responses.

I'd love to have a library that does mediator first and then transparently goes up the chain to distributed requests automatically.

I think you need to do this in your code yourself. If it is possible to make an example for this package in https://github.com/martinothamar/Mediator/tree/main/samples

@R4ND3LL
Copy link

R4ND3LL commented Jul 12, 2024

Allow handlers to be internal - i.e. messages are public contracts, handlers are hidden

Is this feature implemented in the preview yet? If so, how can it be done?

Without using internals visible to, it would make sense to have a source generated mediator within each library containing internal handlers, that way there is no visibility issue, however the root IMediator would still have to coordinate the execution.

@Foxtrek64
Copy link

Another thing I've considered: should this library be renamed? "Mediator" is kind of plain/vanilla. It's basically a working title that I never changed. Feel free to drop suggestions and feedback..

Martiator 👍

@portal7
Copy link

portal7 commented Nov 10, 2024

Searching for implementation tips as a developer can be challenging without relying on Google. Two common issues often arise:

Searches for design patterns frequently lead to MediatR, even when you're looking for general information about the mediator pattern.

Search engines prioritize MediatR results, assuming you need details about using a mediator in C#, which circles back to MediatR.

This makes it easy to fall into a MediatR loop unintentionally.

Renaming the package to a more distinct name would be ideal to avoid this confusion.

@Foxtrek64
Copy link

Foxtrek64 commented Nov 11, 2024

Jokes aside, I agree with @portal7. I too often find a lot of search results end up suggesting things regarding MediatR. DuckDuckGo tends to be a little bit better, but then a lot of articles talking about the mediator pattern end up using MediatR so we're right back there.

I am awful at naming things, but here's a few ideas I came up with:

  • FastMediator (self-descriptive, and distinct from MediatR)
  • M# or MediatorSharp (a bit cliche perhaps)
  • Sprinter (comes from the idea of C# liking to rename types, like Futures being Tasks. I imagine the mediator pattern might be called the "Relay" pattern, because we are relaying information via commands or requests to cause something to happen. A sprinter runs a relay, and this library is also intended to run the relay service, though "run" here is being used in two different senses.)

@codymullins
Copy link

I see it's been a bit since updates for 3.0 preview (no judgment). Any news on how stable or not stable it is to use vs. using 2.x?

@gfoidl
Copy link

gfoidl commented Nov 24, 2024

how stable

In one (quite big) project I use it in production w/o any problems. So I'd say it's stable (didn't encounter any errors neither during build nor at runtime).

Update from 2.x -> 3 was easy, just changes in the method-signature because of CancellationToken moved last (as recommended) in the behaviors.

@martinothamar
Copy link
Owner Author

Since 3.0 has taken so much time, there's a good chance #48 will be the last piece of work going into it. And last I worked on that it's not a perfect design (IMO that is still interceptors), but had good results last I checked. 4.0 will probably/hopefully be based on interceptors and some bigger changes. Are interceptors still preview or were they stabilized as part of 9?

@neozhu
Copy link

neozhu commented Nov 26, 2024

Thanks so much for the effort you've put into this! I'm currently using preview 27 on dotnet 9 and haven't encountered any issues so far. Everything works great on my end. Looking forward to seeing the release new version soon!

@gfoidl
Copy link

gfoidl commented Nov 26, 2024

Are interceptors still preview or were they stabilized as part of 9?

It's something halfway in between...

ASP.NET Core uses interceptors in production, so I'd say they're ready to use (with .NET 9).

@KennethHoff
Copy link

ASP.NET Core uses interceptors in production, so I'd say they're ready to use (with .NET 9).

To be fair, they were also using them in .Net 8, so it's not that good of an argument.

Interceptors IIRC were changed quite a lot in .Net 9, so it's still in flux.

@codymullins
Copy link

Thanks for the feedback. I've been using the preview version in the app we're planning to launch shortly and it seems to be functioning well.

@ejsmith
Copy link

ejsmith commented Nov 27, 2024

I'm confused on trying to figure out if this project is dead or not. :-)

@KANekT
Copy link

KANekT commented Nov 28, 2024

I also don't understand the status of the project - no updates for 6 months...

I found alternative https://immediateplatform.dev/

@martinothamar
Copy link
Owner Author

I don't blame you, I guess this comes down to definitions and expectations 😉

I found alternative https://immediateplatform.dev/

Looks cool! Better in benchmarks, need to look into that, maybe the API/design allows for better perf. Will make sure to compare before completing 3.0

@ziongh
Copy link

ziongh commented Dec 2, 2024

We could use something like Pure.DI, so we can do a complete source generation solution for the injection of the handlers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests