From 82187bfb6b54fdffc5dadaa56e8bf97d2209708a Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Wed, 14 Oct 2020 21:03:11 -0700 Subject: [PATCH 1/2] Dispatch events even when there are no listeners Summary: ## Rationale For every 1 call to RCTNetworking.sendRequest, we execute 6 calls to RCTNetworking.addListener. This is followed by at least one call to RCTNetworking.removeListeners. Aside from incrementing and decrementing the `_listeners` integer, these two methods accomplish nothing else: RCTNetworking doesn't implement the `startObserving` and `stopObserving` methods. This diff makes RCTEventEmitter dispatch events without looking at the listeners integer. In the future, this will allow us to stop making these ~8 unnecessary NativeModule calls for every Network request we send. Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D24272560 fbshipit-source-id: 7996eba5abfa4669a89c43a3ffa536c0faa214a8 --- Libraries/Network/RCTNetworking.mm | 2 +- React/Modules/RCTEventEmitter.h | 2 ++ React/Modules/RCTEventEmitter.m | 27 +++++++++++++++++++++++++-- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/Libraries/Network/RCTNetworking.mm b/Libraries/Network/RCTNetworking.mm index c6fbe7092a72ba..edbbe814e6e57f 100644 --- a/Libraries/Network/RCTNetworking.mm +++ b/Libraries/Network/RCTNetworking.mm @@ -159,7 +159,7 @@ @implementation RCTNetworking - (instancetype)initWithHandlersProvider:(NSArray> * (^)(void))getHandlers { - if (self = [super init]) { + if (self = [super initWithDisabledObservation]) { _handlersProvider = getHandlers; } return self; diff --git a/React/Modules/RCTEventEmitter.h b/React/Modules/RCTEventEmitter.h index f04d50f02418e4..f72f53909f4ba5 100644 --- a/React/Modules/RCTEventEmitter.h +++ b/React/Modules/RCTEventEmitter.h @@ -16,6 +16,8 @@ @property (nonatomic, weak) RCTBridge *bridge; +- (instancetype)initWithDisabledObservation; + /** * Override this method to return an array of supported event names. Attempting * to observe or send an event that isn't included in this list will result in diff --git a/React/Modules/RCTEventEmitter.m b/React/Modules/RCTEventEmitter.m index 6e5a6ff37e7f5d..c3cc596e13ca18 100644 --- a/React/Modules/RCTEventEmitter.m +++ b/React/Modules/RCTEventEmitter.m @@ -12,6 +12,7 @@ @implementation RCTEventEmitter { NSInteger _listenerCount; + BOOL _observationDisabled; } @synthesize invokeJS = _invokeJS; @@ -32,6 +33,13 @@ + (void)initialize } } +- (instancetype)initWithDisabledObservation +{ + self = [super init]; + _observationDisabled = YES; + return self; +} + - (NSArray *)supportedEvents { return nil; @@ -56,12 +64,15 @@ - (void)sendEventWithName:(NSString *)eventName body:(id)body [self class], [[self supportedEvents] componentsJoinedByString:@"`, `"]); } - if (_listenerCount > 0 && _bridge) { + + BOOL shouldEmitEvent = (_observationDisabled || _listenerCount > 0); + + if (shouldEmitEvent && _bridge) { [_bridge enqueueJSCall:@"RCTDeviceEventEmitter" method:@"emit" args:body ? @[ eventName, body ] : @[ eventName ] completion:NULL]; - } else if (_listenerCount > 0 && _invokeJS) { + } else if (shouldEmitEvent && _invokeJS) { _invokeJS(@"RCTDeviceEventEmitter", @"emit", body ? @[ eventName, body ] : @[ eventName ]); } else { RCTLogWarn(@"Sending `%@` with no listeners registered.", eventName); @@ -80,6 +91,10 @@ - (void)stopObserving - (void)invalidate { + if (_observationDisabled) { + return; + } + if (_listenerCount > 0) { [self stopObserving]; } @@ -87,6 +102,10 @@ - (void)invalidate RCT_EXPORT_METHOD(addListener : (NSString *)eventName) { + if (_observationDisabled) { + return; + } + if (RCT_DEBUG && ![[self supportedEvents] containsObject:eventName]) { RCTLogError( @"`%@` is not a supported event type for %@. Supported events are: `%@`", @@ -102,6 +121,10 @@ - (void)invalidate RCT_EXPORT_METHOD(removeListeners : (double)count) { + if (_observationDisabled) { + return; + } + int currentCount = (int)count; if (RCT_DEBUG && currentCount > _listenerCount) { RCTLogError(@"Attempted to remove more %@ listeners than added", [self class]); From dabca52f77799bcdedb6b0ec44b1f6297483a46d Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Wed, 14 Oct 2020 21:03:11 -0700 Subject: [PATCH 2/2] Stop calling RCTNetworking.(add|remove)Listeners? Summary: RCTNetworking.startObserving and RCTNetworking.stopObserving don't exist. The main purpose of RCTEventEmitter.addListener is to call these methods, and increment the `_listeners` counter, so that we can start dispatching events when `_listeners > 0`. In D24272560, I made RCTEventEmitter dispatch events even when _listeners <= 0. This is sufficient for us to stop calling these two RCTNetworking methods entirely. Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D24272663 fbshipit-source-id: de9c968bc71e6e6d69a22b934644e6dfa3266b3f --- Libraries/EventEmitter/NativeEventEmitter.js | 22 ++++++++++++++++---- Libraries/Network/RCTNetworking.ios.js | 9 +++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/Libraries/EventEmitter/NativeEventEmitter.js b/Libraries/EventEmitter/NativeEventEmitter.js index e04bc5f95d06c5..298a3eea2f8831 100644 --- a/Libraries/EventEmitter/NativeEventEmitter.js +++ b/Libraries/EventEmitter/NativeEventEmitter.js @@ -22,15 +22,29 @@ type NativeModule = { ... }; +type NativeEventEmitterOptions = $ReadOnly<{| + __SECRET_DISABLE_CALLS_INTO_MODULE_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: boolean, +|}>; + +const DEFAULT_NATIVE_EVENT_EMITTER_OPTIONS = { + __SECRET_DISABLE_CALLS_INTO_MODULE_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: false, +}; + /** * Abstract base class for implementing event-emitting modules. This implements * a subset of the standard EventEmitter node module API. */ export default class NativeEventEmitter extends EventEmitter { _nativeModule: ?NativeModule; + _disableCallsIntoModule: boolean; - constructor(nativeModule: ?NativeModule) { + constructor( + nativeModule: ?NativeModule, + options: NativeEventEmitterOptions = DEFAULT_NATIVE_EVENT_EMITTER_OPTIONS, + ) { super(RCTDeviceEventEmitter.sharedSubscriber); + this._disableCallsIntoModule = + options.__SECRET_DISABLE_CALLS_INTO_MODULE_DO_NOT_USE_OR_YOU_WILL_BE_FIRED; if (Platform.OS === 'ios') { invariant(nativeModule, 'Native module cannot be null.'); this._nativeModule = nativeModule; @@ -42,7 +56,7 @@ export default class NativeEventEmitter extends EventEmitter { listener: Function, context: ?Object, ): EventSubscription { - if (this._nativeModule != null) { + if (this._nativeModule != null && !this._disableCallsIntoModule) { this._nativeModule.addListener(eventType); } return super.addListener(eventType, listener, context); @@ -51,14 +65,14 @@ export default class NativeEventEmitter extends EventEmitter { removeAllListeners(eventType: string) { invariant(eventType, 'eventType argument is required.'); const count = this.listenerCount(eventType); - if (this._nativeModule != null) { + if (this._nativeModule != null && !this._disableCallsIntoModule) { this._nativeModule.removeListeners(count); } super.removeAllListeners(eventType); } removeSubscription(subscription: EventSubscription) { - if (this._nativeModule != null) { + if (this._nativeModule != null && !this._disableCallsIntoModule) { this._nativeModule.removeListeners(1); } super.removeSubscription(subscription); diff --git a/Libraries/Network/RCTNetworking.ios.js b/Libraries/Network/RCTNetworking.ios.js index 92ef33394316a3..bc1081aab357d6 100644 --- a/Libraries/Network/RCTNetworking.ios.js +++ b/Libraries/Network/RCTNetworking.ios.js @@ -18,7 +18,14 @@ import type {RequestBody} from './convertRequestBody'; class RCTNetworking extends NativeEventEmitter { constructor() { - super(NativeNetworkingIOS); + const disableCallsIntoModule = + typeof global.__disableRCTNetworkingExtraneousModuleCalls === 'function' + ? global.__disableRCTNetworkingExtraneousModuleCalls() + : false; + + super(NativeNetworkingIOS, { + __SECRET_DISABLE_CALLS_INTO_MODULE_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: disableCallsIntoModule, + }); } sendRequest(