fix(native, ios): fix bridgeless mode compatibility for native ad views#845
fix(native, ios): fix bridgeless mode compatibility for native ad views#845adsellor wants to merge 1 commit intoinvertase:mainfrom
Conversation
|
To view this pull requests documentation preview, visit the following URL: docs.page/invertase/react-native-google-mobile-ads~845 Documentation is deployed and generated using docs.page. |
There was a problem hiding this comment.
Code Review
This pull request introduces a singleton pattern for RNGoogleMobileAdsNativeModule to support Bridgeless mode and fixes a property type mismatch in RNGoogleMobileAdsMediaView. The review feedback identifies potential issues with the singleton implementation in multi-instance scenarios and suggests lazy module resolution to prevent nil references during the lazy initialization process characteristic of Bridgeless mode.
| - (instancetype)init { | ||
| if (self = [super init]) { | ||
| _adHolders = [NSMutableDictionary dictionary]; | ||
| _sharedInstance = self; |
There was a problem hiding this comment.
In scenarios with multiple React Native instances (e.g., multiple bridges or surfaces), this assignment will overwrite the _sharedInstance with the most recently initialized module. This can cause views in older instances to incorrectly reference the module from the new instance, potentially leading to state inconsistencies or missing ad references. While acceptable as a workaround for bridgeless mode, consider if a more robust resolution mechanism (like using the module registry) is needed for multi-instance support.
| if (_bridge) { | ||
| _nativeModule = [_bridge moduleForClass:RNGoogleMobileAdsNativeModule.class]; | ||
| } else { | ||
| _nativeModule = [RNGoogleMobileAdsNativeModule sharedInstance]; | ||
| } |
There was a problem hiding this comment.
In Bridgeless mode, TurboModules are often initialized lazily. If this view is instantiated before the RNGoogleMobileAdsNativeModule has been accessed from JavaScript, [RNGoogleMobileAdsNativeModule sharedInstance] will return nil, and _nativeModule will remain nil for the lifetime of this view. It is safer to resolve the module lazily when it is first needed (e.g., in setResponseId:) rather than only during initialization.
| if (_bridge) { | ||
| _nativeModule = [_bridge moduleForClass:RNGoogleMobileAdsNativeModule.class]; | ||
| } else { | ||
| _nativeModule = [RNGoogleMobileAdsNativeModule sharedInstance]; | ||
| } |
There was a problem hiding this comment.
In Bridgeless mode, TurboModules are often initialized lazily. If this view is instantiated before the RNGoogleMobileAdsNativeModule has been accessed from JavaScript, [RNGoogleMobileAdsNativeModule sharedInstance] will return nil, and _nativeModule will remain nil for the lifetime of this view. It is safer to resolve the module lazily when it is first needed (e.g., in setResponseId:) rather than only during initialization.
5e0c83b to
a032bcb
Compare
Description
Fix iOS native ad views crashing/not working in bridgeless (New Architecture) mode.
RNGoogleMobileAdsMediaView(BannerViewProps→MediaViewProps)sharedInstanceweak singleton accessor toRNGoogleMobileAdsNativeModulefor bridgeless module resolutionRNGoogleMobileAdsNativeViewandRNGoogleMobileAdsMediaViewto fall back tosharedInstancewhenRCTBridgeis nilNote
The weak singleton approach is just a workaround to allow usage with new architecture. A future improvement would be to use
RCTModuleRegistryfor proper bridgeless module resolution.Related issues
N/A
Release Summary
iOS native ad views now work correctly in bridgeless (New Architecture) mode.
Checklist
and followed the process outlined there for submitting PRs.
AndroidiOSe2etests added or updated in__tests__e2e__jesttests added or updated in__tests__Test Plan
GADMediaViewrenders media content properly in both modes🔥