-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Trimming] Make it possible to trim controls and handlers #28115
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
Conversation
jsuarezruiz
left a comment
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.
UITests step failing because:
C:\a\_work\1\s\src\Controls\src\Core\Platform\GestureManager\GesturePlatformManager.Windows.cs(91,41): error CS8602: Dereference of a possibly null reference. [C:\a\_work\1\s\src\Controls\src\Core\Controls.Core.csproj::TargetFramework=net9.0-windows10.0.20348.0]
| protected override IElementHandler? GetHandler(IMauiContext context) | ||
| { | ||
| RemapForControlsIfNeeded(); | ||
| return new CheckBoxHandler(); | ||
| } | ||
|
|
||
| [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] | ||
| protected override Type? GetHandlerType() | ||
| { | ||
| RemapForControlsIfNeeded(); | ||
| return typeof(CheckBoxHandler); | ||
| } |
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.
If the trimmer sees the new CheckBoxHandler() call above, is the [return:DAM] GetHandlerType() method needed at all?
Maybe I'm not following what calls GetHandlerType().
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.
I hope we could get rid of the whole method later. For now, I'm keeping it to implement MauiHandlersFactory and make the trimmer happy here:
maui/src/Core/src/Platform/ElementExtensions.cs
Lines 37 to 55 in 11ea51a
| static IElementHandler? CreateTypeWithInjection(this IElement view, IMauiContext mauiContext) | |
| { | |
| var handlerType = mauiContext.Handlers.GetHandlerType(view); | |
| if (handlerType == null) | |
| return null; | |
| #if ANDROID | |
| if(mauiContext.Context != null) | |
| { | |
| return (IElementHandler)Extensions.DependencyInjection. | |
| ActivatorUtilities.CreateInstance(mauiContext.Services, handlerType, mauiContext.Context); | |
| } | |
| #endif | |
| return (IElementHandler)Extensions.DependencyInjection. | |
| ActivatorUtilities.CreateInstance(mauiContext.Services, handlerType); | |
| } | |
9c90cfd to
fd9d57a
Compare
Description of Change
This proof of concept explores how we could get rid of the long DI setup code in AppHostBuilderExtensions:
Listing all the classes this way makes it impossible for the trimmer to remove any of them.
The idea in this PR is to change
MauiHandlersFactoryto return the handler instance or the handler type from by calling an instance method on the control instead of looking up the handler in the DI container:GetHandlerandGetHandlerTypeThe DI container should still be considered first before calling the instance method on the view for two reasons:
Notes:
RemapForControlswould be handled.LabelandCheckBox. These two are chosen becauseLabelis in thedotnet new mauiapp and so we can test that the handler for that control is created correctly, andCheckBoxis NOT in the sample app, so we can verify that the trimmer removes this class + theCheckBoxHandlerwhen trim mode is set to full.Feedback is very welcome!
/cc @jonathanpeppers @PureWeen @mattleibow @ivanpovazan @vitek-karas