jbogard / MediatR

Simple, unambitious mediator implementation in .NET
Apache License 2.0
11k stars 1.16k forks source link

Skip unnecessary BCL package reference if targeting .NET 6 or later #857

Closed asimmon closed 1 year ago

asimmon commented 1 year ago

As mentioned in #852, I would like to introduce multi-targeting only to skip an unnecessary package reference (Microsoft.Bcl.AsyncInterfaces) on .NET 6 and later, because it's already included by default.

A (positive?) side effect of targeting net6.0 is that a few warnings appeared in existing code because of new nullable reference types annotations. For instance, IServiceProvider.GetService(Type serviceType):

At runtime, the behavior is the same. It's just that Microsoft added more hints to prevent null reference exceptions. I had to fix these warnings, and that explains the number of changes.

While fixing these warnings in Mediator.cs, I also added:

asimmon commented 1 year ago

😕Seems like there is a flaky test that relies on timing (MediatR.Tests.NotificationPublisherTests.Should_handle_sequentially_by_default). I guess that this is to test parallel notification handlers against sequential notification handlers, but on GitHub agents this can be unreliable.

asimmon commented 1 year ago

Were the additional null checks a side effect of .NET 6?

Yes. Basically it adds more precise hints on the existing runtime behavior. I think it's a positive thing, it puts in evidence paths that could eventually throw a null-reference exception. GetService is a good example. In the current master state, it could return null but there's no warning whatsoever.