-
Notifications
You must be signed in to change notification settings - Fork 86
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
Support generic imports (GenericHost/Options DI) #457
Comments
It's an interesting idea, but we have no plans to support this at this point. Open generic exports are easier to support because the generic type parameter can be ignored as it doesn't impact the composition graph. Generic imports on the other hand require a different representation in the composition for every anticipated type argument. I suppose supporting it is possible, but it's more difficult. If I assume that System.ComponentModel.Composition's MEF engine supported this, I'd argue it's easier because they don't keep a static graph -- theirs is dynamic. They only figure out what a MEF part needs when it's activated. And they do it again the next time it's activated. It's why that MEF engine is so slow. I believe a dynamic component wouldn't be necessary to support this, now that I think about it. But I don't see us funding the work anytime in the future unless it became critical for a VS feature. |
I actually didn't consider checking
I don't believe this is any more dynamic than the existing open generic exports and this is mostly a question how to represent these dependencies. (That doesn't mean its easy, its probably a significant challenge, generics unfortunately aren't trivial.)
I assume that also means contributions of this kind are currently not desired either, as they likely will cause additional effort required to review and align with design of the library? This was basically my main question, whether there's any interest in exploring how to add this feature, not necessarily asking you to provide it. I understand that it may be disruptive to the workings of the library and require effort for feedback/communications, so if this kind of high effort contributions are currently not desired I perfectly understand. Just wanted to ask before I invest any work myself, so far I have just identified the problem and no idea how a solution would look like. Feel free to close as "not planned" or similar in this case, and thanks for the feedback 👍 |
I think we'd take the feature if you offered a pull request that met the requirements we have for all our new features. Most importantly:
I bet we even have a test or two for this feature already, but disabled for the VS-MEF engine. So part of the work would be to look through our VS-MEF disabled tests to see if any apply that should be enabled. That would be easier than writing new tests, though depending on the coverage these offered, additional tests may (probably will) be needed. Thanks for thinking about it! |
We've had Generic Host for a while now and I've been looking into using vs-mef as DI instead of the inbox implementation, to integrate better with existing codebases which already are MEF based. I've made good progress mapping the builder abstractions and constructing a dynamic vs-mef catalog from ServiceDescriptors during startup, but when I got to the options infrastructure of .NET Core I'm running into the problem that they adopted generic imports, something vs-mef doesn't support yet.
An example for exports added to the builder is here:
So .NET is creating generic exports, which is fine, vs-mef supports that. However when we look ad e.g.
UnnamedOptionsManager<TOptions>
we can see its imports:i.e. the import depends on the generic parameter, which is something vs-mef does not support because the TypeRef infrastructure has no strategy to deal with it and just throws.
Conceptually I believe it should be fine if a generic export imports another generic export with a matching parameter? So the question is if there is any desire to have this support added to vs-mef.
The text was updated successfully, but these errors were encountered: