fix(android): stop isDebug() reaching ReactNativeHost (fatal on New Architecture) - #8341
Open
ninjz wants to merge 2 commits into
Open
fix(android): stop isDebug() reaching ReactNativeHost (fatal on New Architecture)#8341ninjz wants to merge 2 commits into
ninjz wants to merge 2 commits into
Conversation
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
Context.isDebug()readsuseDeveloperSupportoff the legacyReactNativeHost:Under the New Architecture that host no longer exists — an app overrides
reactHostinstead, andReactApplication.getReactNativeHost()'s default getter throws by design:So on New Arch every call to
isDebug()is fatal. This replaces the body with a check that needs no React APIs at all.How it's actually hit: icons loaded by URI
The crash is easy to miss because it is gated behind a short-circuit in
ImageLoader.getDrawable:context.isDebug()is only reached whenloadResource()fails — i.e. when the icon is not bundled in the APK as an Android drawable resource, and has to be fetched from the Metro dev server instead. That is exactly what happens for icons supplied by URI (require('./icon.png')resolved through Metro in a debug build).That produces a confusing split:
loadResource()isDebug()called?nullSo apps that bundle their tab icons as drawable resources never see this, and release builds never see it. An app on New Arch that loads icons by URI hard-crashes the moment a
bottomTabsroot is set — which makes everything behind the tab bar unreachable in development.Stack trace (react-native-navigation 8.8.11, react-native 0.85.3,
newArchEnabled=true, Android API 36):The tab-icon path is just the one that detonates first.
isDebug()has four call sites, all equally fatal on New Arch, which is why this fixes the function rather than the caller:utils/Context.ktutils/ImageLoader.ktgetDrawable— the tab-icon crashutils/ImageLoader.ktreadJsDevImageutils/ImageLoader.ktadjustThreadPolicyDebugviewcontrollers/viewcontroller/YellowBoxDelegate.ktonChildViewAddedearly-returnThe change
FLAG_DEBUGGABLEanswers the same question the three call sites are actually asking — "is this a development build, so is it OK to fetch an image from a dev server / relax StrictMode / expect LogBox views?" — without depending on either React host. It works identically on both architectures, so no branching is required.It also removes the last use of
com.facebook.react.ReactApplicationin this file, so the import goes with it.Behaviour: for a standard setup
useDeveloperSupportdefaults to the debug build type, which is exactly whatFLAG_DEBUGGABLEreports — so this is equivalent on Old Arch and correct rather than fatal on New Arch. The one nuance worth flagging: an app that deliberately overrodegetUseDeveloperSupport()to something other thanBuildConfig.DEBUGwould now seeisDebug()follow the manifest's debuggable flag instead. Happy to switch to readingreactHost.devSupportEnabledwith a fallback if you'd prefer to preserve that, though it's a good deal more code for a case these three call sites don't really care about.iOS is unaffected —
isDebugexists only in the Android source.ImageParser.mmhands URI icons to[RCTConvert UIImage:], which already resolves dev-server URLs, so iOS never needs the check.Verification
Tested on a real app (react-native 0.85.3, RNN 8.8.11, New Arch, Android API 36 emulator), same commit both ways with the patch as the only variable:
FATAL EXCEPTIONonsetRootas the tab bar is constructed, process dies, 100% reproducibleThe icons rendering is the part that confirms the replacement is correct and not merely non-fatal: they only appear if
isDebug()returnedtrueand sent the loader downreadJsDevImage()to fetch them from Metro. A wrong-but-safe implementation would have produced blank icons instead.Release builds were verified unchanged.