[Trimming] Use new feature switch definition attribute and enable analyzers in Controls.Core.csproj #21621
Conversation
|
@sbomer for both review and as an FYI. |
|
@sbomer I was trying to understand the |
There was a problem hiding this comment.
LGTM! If you are using the trim analyzer and would like IsXamlRuntimeParsingSupported to guard RequiresUnreferencedCodce-annotated code, you can also add [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] to the property.
You'll need to suppress IL4000 (which is emitted when the analyzer can't see that the property will always be false when unreferenced code is removed) on the property, and ensure manually that the feature switch is set to false in a trimmed app.
| @@ -20,6 +20,9 @@ internal static class RuntimeFeature | |||
| private const bool IsQueryPropertyAttributeSupportedByDefault = true; | |||
| private const bool IsImplicitCastOperatorsUsageViaReflectionSupportedByDefault = true; | |||
|
|
|||
| #if !NETSTANDARD | |||
| [FeatureSwitchDefinition("Microsoft.Maui.RuntimeFeature.IsXamlRuntimeParsingSupported")] | |||
There was a problem hiding this comment.
Just wanted to check that it's expected for the feature switch to no longer work in the netstandard build.
There was a problem hiding this comment.
Yes, that's expected. The netstandard build doesn't need trimming, only the platform specific builds do.
|
@sbomer thanks for the additional details. I tried what you say before and I was confused by the IL4000 and I thought I must be using it incorrectly. I'll give it another go. |
| @@ -8,6 +9,7 @@ | |||
| namespace Microsoft.Maui.Controls | |||
| { | |||
| /// <include file="../../docs/Microsoft.Maui.Controls/VisualElement.xml" path="Type[@FullName='Microsoft.Maui.Controls.VisualElement']/Docs/*" /> | |||
| [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields)] | |||
There was a problem hiding this comment.
I'm thinking about putting CSS styling behind a feature flag instead of preserving all public and non-public fields on all VisualElement classes. Would it be acceptable to say that CSS isn't supported with trimming?
On the other hand, when I compared the .ipa size of NativeAOT app build with this [DAM] and without it, the size difference was negligible. I worry that when somebody uses Telerik or some other control library, it might suddenly become more pronounced.
| [UnconditionalSuppressMessage("Trimming", "IL2075", Justification = TrimmerConstants.NativeBindingService)] | ||
| [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = TrimmerConstants.NativeBindingService)] |
There was a problem hiding this comment.
I'm not very happy we have these UnconditionalSuppressMessage here. I think in this case it might be worth putting [RequiresUnreferencedCode] on the INativeBindingService.TrySetBinding(object, string, BindingBase) and all the classes that implement it.
Description of Change
We can use
[FeatureSwitchDefinition("...")]attributes instead ofILLink.Substitutions.xmlsince dotnet/runtime#99338 has been merged and has flown into MAUI./cc @vitek-karas
Issues Fixed
Contributes to #18658