-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix incorrect NativeAOT warnings #4242
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
base: main
Are you sure you want to change the base?
Conversation
…I target - Prevents most of WhenAnyValue warnings from non WinUI targets
8080127 to
18cf0be
Compare
…y on WinUI target
7a120eb to
33b2be7
Compare
|
Request a discussion before opening a PR since you can only tell if the warning is invalid the AOT test app. Closing for the moment just to avoid you wasting your time unnecessarily |
|
I am happy to hear that now there will be a focus on NativeAOT support for ReactiveUI. I want to mention that my last known information was this one from #4018:
This PR neither had the goal to make ReactiveUI fully AOT compatible nor to fix all incorrect warnings. My goal simply was to resolve the warnings for the most frequently used methods so consumers like me do not have to annotate the own codebase with thousands of UnconditionalSuppress annotations. In order to be able to use the recent version of ReactiveUI, I hoped that I can contribute something that can be used in short-term in order to not need to wait until ReactiveUI is rewritten from scratch to provide bullet-proof AOT-support. I already suspected that I may be missing a correct way to identify invalid warnings. Because of this, I asked for help in the linked discussion. But again - I am very pleased that you will make ReactiveUI to a library with AOT implemented from scratch including source generators. So good 🎉 And thank you for clarifying this in order to prevent waste of time. As this would have been my first contribution to ReactiveUI, I invested already a few hours in order to get everything running and being able to provide a PR draft. |
|
No Problem, definitely my fault. I did mean to respond to your earlier query at the time but got side tracked, We are normally very open to any contribution from the community, just very tough when you get a PR that doesn't quite fit with our plans (and to be honest removing the excessive warnings is a very worthy one), you don't want to put people off contributing to the project since we can definitely use all the help we can get also. |
|
Been thinking about it, as a stop gap measure might be worthwhile if we just do the warning removal. |
|
I might reopen a separate branch and just attribute the fixes to you (do co-author-by), might be easier since I been actively working on all this. Will use the majority of the work you been using. |
|
Nice to hear that 👍
If the specific implementation for WinUI is optional, it might be possible to make WhenAnyValue & Co. completely NativeAot safe.
|
|
Step one is reactiveui/splat#1461 which removes a lot of the down stream warnings. |
Issue and discussion references
What kind of change does this PR introduce?
The Pull-Request tries to eliminate incorrect generation of NativeAOT and Trimming warnings for some frequently used ReactiveUI methods (like
WhenAnyValue)What is the current behavior?
Lots of incorrect IL2026 and IL3050 warnings when using ReactiveUI with NativeAOT or Trimming enabled in an project.
What is the new behavior?
Less incorrect of these warnings.
What might this PR break?
This PR shouldn't break anything as pretty all changes just fix incorrect annotations or add new compiler hints in order to let the trimmer work correctly.
Please check if the PR fulfills these requirements
Other information:
State of the Pull Request Draft:
Fix incorrect warnings for
WhenAnyValueon non WinUI PlatformsICreatesObservableForPropertyis registered which needsRequiresUnreferencedCodeNew API: AOT-friendly
void Reflection.ThrowIfMethodsNotOverloaded(string callingTypeName, Type targetType, params string[] methodsToCheck)methodFix incorrect warnings for
ReactiveCommand.Create...