-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fixes missing dependencies in the ShellBlueprint #16379
Conversation
…hey are not related to a specific feature. Also fixes LoadFeaturesAsync() to return the ordered features list.
This pull request has merge conflicts. Please resolve those before requesting a review. |
Does this need triage @MikeAlhayek or can be merged? |
This LGTM. But another set of eye may be great. I trust @gvkries |
Let's check this on Thursday together. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see if we can get an easy test for that.
Let's merge it, then. |
If a module contains no feature that matches the extension ID, some dependencies like startup classes were not being added to the
ShellBlueprint
and therefore not executed at all.This PR fixes that by changing the
ExtensionManager
to always add every exported type to the features contained in the module. Any type that had no associated feature before, is now added to each feature as a dependency. By doing this, theCompositionStrategy
will correctly find all dependencies and all startup classes are executed.Note that while working on this fix, I found another issue. The order in which startup classes are executed depends on the
ExtensionManager.LoadFeaturesAsync()
method, which must already return the features in order of dependencies. However, it returns an unordered list that comes from theDictionary.Values
property. I've fixed that as well, which allows us to use anotherFrozenDictionary
as a small tweak as well.This PR is based on #16324 because this change is in the same area and there is no benefit to fixing before #16324.
Fixes #16375