Repository navigation
fix(android): log missing generated setter at debug level, not warn - #58854
Open
franruiztech wants to merge 1 commit into
Open
franruiztech wants to merge 1 commit into
franruiztech wants to merge 1 commit into
Conversation
ViewManagerPropertyUpdater looks up a "<class>$$PropsSetter" class by reflection and falls back to reflection over the @ReactProp methods when it is not found. The annotation processor that generated those setters (ReactPropertyProcessor) was removed in react#49811, so the fallback is now the normal path for every view manager and shadow node. Logging it at warn level prints one "Could not find generated setter for class ..." line per class on every app start, for a condition that is neither an error nor actionable. Lower it to debug. The lookup is kept, since a library can still ship a generated setter.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
ViewManagerPropertyUpdater.findGeneratedSetterlooks up a<class>$$PropsSetterclass by reflection and, when it does not exist, logsCould not find generated setter for class ...at warn level and falls back to reflection over the@ReactPropmethods (FallbackViewManagerSetter/FallbackShadowNodeSetter).The annotation processor that generated those setters (
ReactPropertyProcessor) was removed in #49811 (first released in 0.80). Since then no view manager in React Native has a generated setter, so the fallback is the normal path and the warning is printed once per view manager and shadow node class on every app run (71 lines in a typical app start on 0.85.3). It is not an error and there is nothing the app developer can do about it. Users have reported it as logcat noise several times (#29864, #31416, #45211), and it is easy to mistake for the cause of an unrelated crash.This changes the log call from
FLog.wtoFLog.d. The lookup itself is kept, because a library can still ship a generated setter, and the message stays available for anyone debugging prop setup. It does not touchFLogconfiguration.Changelog:
[ANDROID] [FIXED] - Do not log "Could not find generated setter" at warn level; the reflection fallback is the expected path
Test Plan:
Added
ViewManagerPropertyUpdaterTest: it resolves the native props of a view manager that has no generated setter, withFLogmocked, and checks that the props are still found through the fallback and thatFLog.wis not called.Ran it with
./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests com.facebook.react.uimanager.ViewManagerPropertyUpdaterTest: 1 test, passes. With the call put back toFLog.w(and nothing else changed) the same run fails withNeverWantedButInvokedforFLog.w(String, String), reporting theCould not find generated setter for class ...ViewManagerWithoutGeneratedSettercall fromfindGeneratedSetter. The test follows the samemockStatic(FLog)pattern as the neighbouring tests.I also compiled the changed
ViewManagerPropertyUpdateragainst the 0.85.3react-androidrelease AAR and replaced the classes in it;javap -cshows the call site now targetsFLog.dand nothing else in the class changed.On an Android 9 (API 28) x86_64 emulator, with a release build of an app on 0.85.3 (R8, Hermes, New Architecture), I ran the same scripted flow twice: login, five tabs, a list with about 20 remote logos, a company detail screen, and a cold restart. I counted the matching logcat lines.
react-androidCould not find generated setterStock 0.85.3 on API 26, 31, 33 and 36 gives 141 to 142 lines for the same flow. The patched build was run on API 28 and API 34 (Android 14), with 0 lines on both, and it did not crash.
The patched
react-androidwas built by recompiling the changed classes and replacing them in the official 0.85.3 AAR, not with a full Gradle build ofReactAndroid.