Uh oh!
There was an error while loading. Please reload this page.
BREAKING - RCTEvent improvements, remove deprecated [sendInputEventWithName:body:]#15894
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| /** | ||
| * Copyright (c) Facebook, Inc. and its affilities. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
| #import <React/RCTEventDispatcher.h> | ||
| /** | ||
| * Generic untyped event for Components. Used internally by RCTDirectEventBlock and | ||
| * RCTBubblingEventBlock, for other use cases prefer using a class that implements | ||
| * RCTEvent to have a type safe way to initialize it. | ||
| */ | ||
| @interface RCTComponentEvent : NSObject<RCTEvent> | ||
| - (instancetype)initWithName:(NSString *)name viewTag:(NSNumber *)viewTag body:(NSDictionary *)body; | ||
Contributor There was a problem hiding this comment. We need to add some nonnull attributes to prevent crash? ContributorAuthor There was a problem hiding this comment. This file could use | ||
| @end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| /** | ||
| * Copyright (c) Facebook, Inc. and its affilities. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
| #import "RCTComponentEvent.h" | ||
| #import "RCTAssert.h" | ||
| @implementation RCTComponentEvent | ||
| { | ||
| NSArray *_arguments; | ||
| } | ||
| @synthesize eventName = _eventName; | ||
| @synthesize viewTag = _viewTag; | ||
| - (instancetype)initWithName:(NSString *)name viewTag:(NSNumber *)viewTag body:(NSDictionary *)body | ||
| { | ||
| if (self = [super init]) { | ||
| NSMutableDictionary *mutableBody = [NSMutableDictionary dictionaryWithDictionary:body]; | ||
Contributor There was a problem hiding this comment. Wait, that does not look right. Why do we need put tag inside body? That was needed for old poorly designed ContributorAuthor There was a problem hiding this comment. I'm pretty sure JS needs the tag in the body that's why we put it here. I could be wrong though I'd have to look at the RCTEventEmitter.receiveEvent code. ContributorAuthor There was a problem hiding this comment. Yea it is definitely used https://github.com/facebook/react/blob/7f78749ee3023608a614b2f595739bcd36acc128/src/renderers/native/ReactNativeEventEmitter.js#L112. I'd rather not touch this :) Contributor There was a problem hiding this comment. Yeah... But... I am trying understand that code... and I failed. ContributorAuthor There was a problem hiding this comment. Yea I'm not sure why we are sending the tag as both the first arg and in the event body as target. I just think we should keep it like this to avoid breaking react. ContributorAuthor There was a problem hiding this comment. @shergin Let's see if someone from the react team can help here. I agree we should have a clear plan here. We also have to consider android if we make any change to the native / react bridging api. Contributor There was a problem hiding this comment. Hi! I'm only somewhat familiar with our event code. (I don't think anyone on the core team presently is deeply familiar with it, to be honest.) The event params like If the 2 values are always the same (for Android too) then it seems silly to always pass them both and I could update the Contributor There was a problem hiding this comment. FWIW it looks like the Java implementation doesn't necessarily pass a ContributorAuthor There was a problem hiding this comment. Thanks @bvaughn, I'm pretty sure we need to keep target in the event payload otherwise it would be a breaking change for apps that rely on this value. Maybe we could avoir passing the tag as the first argument if it's always the same as body.target. Needs more investigation. @shergin What do you think about landing this as is since it will unblock a few other improvements I wanted to make? Contributor There was a problem hiding this comment.
Sure! To be clear: I wasn't suggesting we remove But that's obviously kind of a polish detail and could be done, or not, at any time. | ||
| mutableBody[@"target"] = viewTag; | ||
| _eventName = RCTNormalizeInputEventName(name); | ||
| _viewTag = viewTag; | ||
| _arguments = @[_viewTag, _eventName, mutableBody]; | ||
| } | ||
| return self; | ||
| } | ||
| RCT_NOT_IMPLEMENTED(- (instancetype)init) | ||
| - (NSArray *)arguments | ||
| { | ||
| return _arguments; | ||
| } | ||
| - (BOOL)canCoalesce | ||
| { | ||
| return NO; | ||
| } | ||
| + (NSString *)moduleDotMethod | ||
| { | ||
| return @"RCTEventEmitter.receiveEvent"; | ||
| } | ||
| @end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -10,8 +10,9 @@ | ||
| #import "RCTAssert.h" | ||
| #import "RCTBridge.h" | ||
| #import "RCTBridge+Private.h" | ||
| #import "RCTComponentEvent.h" | ||
| #import "RCTProfile.h" | ||
| #import "RCTUtils.h" | ||
| const NSInteger RCTTextUpdateLagWarningThreshold = 3; | ||
| @@ -29,7 +30,7 @@ | ||
| static NSNumber *RCTGetEventID(id<RCTEvent> event) | ||
| { | ||
| return @( | ||
| ([event respondsToSelector:@selector(viewTag)] ? event.viewTag.intValue : 0) | | ||
| (((uint64_t)event.eventName.hash & 0xFFFF) << 32) | | ||
| (((uint64_t)event.coalescingKey) << 48) | ||
| ); | ||
| @@ -79,20 +80,6 @@ - (void)sendDeviceEventWithName:(NSString *)name body:(id)body | ||
| completion:NULL]; | ||
| } | ||
| - (void)sendTextEventWithType:(RCTTextEventType)type | ||
| reactTag:(NSNumber *)reactTag | ||
| text:(NSString *)text | ||
| @@ -110,7 +97,6 @@ - (void)sendTextEventWithType:(RCTTextEventType)type | ||
| NSMutableDictionary *body = [[NSMutableDictionary alloc] initWithDictionary:@{ | ||
| @"eventCount": @(eventCount), | ||
| }]; | ||
| if (text) { | ||
| @@ -134,10 +120,10 @@ - (void)sendTextEventWithType:(RCTTextEventType)type | ||
| body[@"key"] = key; | ||
| } | ||
| RCTComponentEvent *event = [[RCTComponentEvent alloc] initWithName:events[type] | ||
| viewTag:reactTag | ||
| body:body]; | ||
| [self sendEvent:event]; | ||
| } | ||
| - (void)sendEvent:(id<RCTEvent>)event | ||
| @@ -149,34 +135,39 @@ - (void)sendEvent:(id<RCTEvent>)event | ||
| } | ||
| [_observersLock unlock]; | ||
| if (event.canCoalesce) { | ||
| [_eventQueueLock lock]; | ||
| NSNumber *eventID = RCTGetEventID(event); | ||
| id<RCTEvent> previousEvent = _events[eventID]; | ||
| if (previousEvent) { | ||
| event = [previousEvent coalesceWithEvent:event]; | ||
janicduplessis marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } else { | ||
| [_eventQueue addObject:eventID]; | ||
| } | ||
| _events[eventID] = event; | ||
| BOOL scheduleEventsDispatch = NO; | ||
| if (!_eventsDispatchScheduled) { | ||
| _eventsDispatchScheduled = YES; | ||
| scheduleEventsDispatch = YES; | ||
| } | ||
| // We have to release the lock before dispatching block with events, | ||
| // since dispatchBlock: can be executed synchronously on the same queue. | ||
| // (This is happening when chrome debugging is turned on.) | ||
| [_eventQueueLock unlock]; | ||
| if (scheduleEventsDispatch) { | ||
| [_bridge dispatchBlock:^{ | ||
| [self flushEventsQueue]; | ||
| } queue:RCTJSThread]; | ||
| } | ||
| } else { | ||
| [_bridge dispatchBlock:^{ | ||
| [self dispatchEvent:event]; | ||
| } queue:RCTJSThread]; | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.