Skip to content

[Looking for feedback] Revamping OnModuleBuilding if we can target .NET 7+ #2525

Open
@Joe4evr

Description

@Joe4evr

It's been a while since I last contributed to D.Net, and I've been thinking for a (long) while on fixing what I feel was quite a mistake in how OnModuleBuilding was put in place. Unfortunately, at the time there wasn't a good way to have members that are 1) easily discoverable, 2) allow derived module types the ability to "adjust some knobs" of their base type, and 3) not require module instantiation. This caused a lot of confusion among users at the time, and I still feel like it's terrible that all modules have to be instantiated in order to cater to it.

But with .NET 7/C# 11 we finally have static abstract members, and they are awesome! (Minus a reflection bug that makes the way I've currently got it all figured out not quite work how I want it. 🙃) For now, though, I'd like to run the API design by folks to get feedback on it. I've put a quick summary of it in this gist: https://gist.github.com/Joe4evr/9e15f823ad7517f1238ee9d019b93145

TL;DR: We expose an interface (or 3) defining a static abstract method that users can put on their module to indicate they want said method called when said module is loaded.

Open questions

  1. Do we want the 3 different interfaces to separate the Basic/Advanced/Expert scenarios, or do we believe it's better to only have the Expert version for maximum flexibility with less complexity of choice?
    • (In that case we're saying there's no point in using this and not having a config interface on your module. TState can always be object if the module author doesn't need anything supplied there.)
  2. The runtime bug I alluded to: The only thing that's blocking is checking if the implementation method found is on the current type while walking the hierarchy. To explain better:
    • In this example, we'd walk the hierarchy and check each type if it happens to implement (one of) the callback interface(s). What normally ends up happening is that all types get a match because SomeBaseModule has it.
    • What I want to do is only execute that method when the current type in hierarchy traversal is SomeBaseModule, which should be easy using GetInterfaceMap, but this method apparently wasn't updated to understand static abstract methods that also have generic constraints, and so it throws VerificationException even though it's valid code.
    • Is this reasoning valuable enough to block the feature on runtime/SDK versions prior to whatever servicing release will fix it, or should we simply let it happen and tell users they need to be fully resilient against multiple invocations for the same module registration?

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions