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

Adding type forwardings for types that were moved to the Contracts package #800

Merged
merged 1 commit into from
Nov 19, 2022

Conversation

ydie22
Copy link
Contributor

@ydie22 ydie22 commented Nov 14, 2022

This pull request is a proposal to solve the issue described in #692

@jbogard
Copy link
Owner

jbogard commented Nov 15, 2022

Is anyone still having this issue? No more comments in many months. I don't want to pull this in if it's not a big issue anymore.

@bend
Copy link

bend commented Nov 15, 2022

@jbogard On my side we still have this issue, this is a blocking us from upgrading to the new version of Mediatr (10 or 11)

@jbogard
Copy link
Owner

jbogard commented Nov 15, 2022

It sounds this issue comes from transitive dependencies to MediatR:

  • App A
    • MediatR 10.0
    • MediatR.Contracts 10.0
    • SomethingElse 1.0
      • MediatR 9.0

If that's the case, then this PR won't fix anything. MediatR contains breaking changes from 9.0 to 10.0 and 10.0 to 11.0. That SomethingElse 1.0 package will break anyway.

The correct solution here is to fix SomethingElse package.

Unless there's some other issue I'm unaware of?

@ydie22
Copy link
Contributor Author

ydie22 commented Nov 15, 2022

Let me explain what situation this PR is supposed to solve.
Say we start with the following situation:

  • AppA
    • AppB.Contracts
      • MediatR 8.1
    • AppC.Contracts
      • MediatR 8.1
        AppA works fine as everyone is using MediatR 8.1 where all types (contract-related and implementation) are still contained in a single assembly.
        Now comes the MediatR version where contract types are separated in another package/assembly (which is btw, an excellent thing). The owners of AppC decide to migrate their MediatR version and they publish a new contracts package. Now AppA needs this new version of the contracts because there is a new feature exposed in the contracts. So they upgrade. The situation becomes:
  • AppA
    • MediatR 11.0
    • AppB.Contracts
      • MediatR 8.1
    • AppC.Contracts
      • MediatR.Contracts 1.0
        Notice that to be able to use the types in AppC.Contracts, AppA must now reference directly MediatR 11.0. And this is where things break. The package version will resolve to MediatR 11.0 and this one will be loaded at runtime. However, when AppB.Contracts was compiled, the MediatR interfaces used on request types it contains were present in the MediatR.dll (version 8.1). And now suddenly, they're no longer there as expected, because MediatR 11.0 does not contain them, they're in MediatR.Contracts. The runtime is lost, it cannot load the request types defined in AppB.Contracts.
        However, if MediatR v11.0 would contain a hint to the runtime that the types he is looking for have moved, he would look at that additional place, and that is exactly what the PR does: it redirects the runtime to the new location where the types have moved. As AppB.Contracts and AppC.Contracts only contain contract definitions, they don't break; they are the kind of packages that MediatR.Contracts typically was designed for.

One could argue that AppB.Contracts should be upgraded to use MediatR.Contracts 1.0 also, but this is irrealistic once you start to have tens of services maintained by different teams. Such a forced synchronized upgrade is then really painful.

@jbogard
Copy link
Owner

jbogard commented Nov 19, 2022

Ah that makes sense, to prevent forced upgrade of all services all at once.

@jbogard jbogard merged commit d2f166e into jbogard:master Nov 19, 2022
@ydie22
Copy link
Contributor Author

ydie22 commented Dec 7, 2022

@jbogard , thank you for merging this PR, it has saved us a lot of time while upgrading!

@ydie22 ydie22 deleted the type-forwarding branch December 7, 2022 19:46
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

Successfully merging this pull request may close these issues.

3 participants