Skip to content

Commit 4e215b2

Browse files
janicduplessisfacebook-github-bot
authored andcommitted
Fix event ordering when combining coalescable and non-coalescable events (#24693)
Summary: Fixes an issue introduced in #15894 that can cause events to be dispatched out of order. Also reverted `viewTag` moved to optional property as it didn't actually work and makes code more complex. [iOS] [Fixed] - Fix event ordering when combining coalescable and non-coalescable events Pull Request resolved: #24693 Differential Revision: D15279513 Pulled By: shergin fbshipit-source-id: 3c64aba6d644ea9564572e6de8330b59b51cf4a9
1 parent 12fb97d commit 4e215b2

2 files changed

Lines changed: 30 additions & 31 deletions

File tree

React/Base/RCTEventDispatcher.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ RCT_EXTERN NSString *RCTNormalizeInputEventName(NSString *eventName);
3535
@protocol RCTEvent <NSObject>
3636
@required
3737

38+
@property (nonatomic, strong, readonly) NSNumber *viewTag;
3839
@property (nonatomic, copy, readonly) NSString *eventName;
3940

4041
- (BOOL)canCoalesce;
@@ -47,12 +48,6 @@ RCT_EXTERN NSString *RCTNormalizeInputEventName(NSString *eventName);
4748

4849
@optional
4950

50-
/**
51-
* Can be implemented for view based events that need to be coalesced
52-
* by it's viewTag.
53-
*/
54-
@property (nonatomic, strong, readonly) NSNumber *viewTag;
55-
5651
/**
5752
* Coalescing related methods must only be implemented if canCoalesce
5853
* returns YES.

React/Base/RCTEventDispatcher.m

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,17 @@
2727
return eventName;
2828
}
2929

30-
static NSNumber *RCTGetEventID(id<RCTEvent> event)
30+
static NSNumber *RCTGetEventID(NSNumber *viewTag, NSString *eventName, uint16_t coalescingKey)
3131
{
3232
return @(
33-
([event respondsToSelector:@selector(viewTag)] ? event.viewTag.intValue : 0) |
34-
(((uint64_t)event.eventName.hash & 0xFFFF) << 32) |
35-
(((uint64_t)event.coalescingKey) << 48)
33+
viewTag.intValue |
34+
(((uint64_t)eventName.hash & 0xFFFF) << 32) |
35+
(((uint64_t)coalescingKey) << 48)
3636
);
3737
}
3838

39+
static uint16_t RCTUniqueCoalescingKeyGenerator = 0;
40+
3941
@implementation RCTEventDispatcher
4042
{
4143
// We need this lock to protect access to _events, _eventQueue and _eventsDispatchScheduled. It's filled in on main thread and consumed on js thread.
@@ -136,38 +138,40 @@ - (void)sendEvent:(id<RCTEvent>)event
136138

137139
[_observersLock unlock];
138140

139-
if (event.canCoalesce) {
140-
[_eventQueueLock lock];
141-
142-
NSNumber *eventID = RCTGetEventID(event);
141+
[_eventQueueLock lock];
143142

143+
NSNumber *eventID;
144+
if (event.canCoalesce) {
145+
eventID = RCTGetEventID(event.viewTag, event.eventName, event.coalescingKey);
144146
id<RCTEvent> previousEvent = _events[eventID];
145147
if (previousEvent) {
146148
event = [previousEvent coalesceWithEvent:event];
147149
} else {
148150
[_eventQueue addObject:eventID];
149151
}
150-
_events[eventID] = event;
152+
} else {
153+
id<RCTEvent> previousEvent = _events[eventID];
154+
eventID = RCTGetEventID(event.viewTag, event.eventName, RCTUniqueCoalescingKeyGenerator++);
155+
RCTAssert(previousEvent == nil, @"Got event %@ which cannot be coalesced, but has the same eventID %@ as the previous event %@", event, eventID, previousEvent);
156+
[_eventQueue addObject:eventID];
157+
}
151158

152-
BOOL scheduleEventsDispatch = NO;
153-
if (!_eventsDispatchScheduled) {
154-
_eventsDispatchScheduled = YES;
155-
scheduleEventsDispatch = YES;
156-
}
159+
_events[eventID] = event;
157160

158-
// We have to release the lock before dispatching block with events,
159-
// since dispatchBlock: can be executed synchronously on the same queue.
160-
// (This is happening when chrome debugging is turned on.)
161-
[_eventQueueLock unlock];
161+
BOOL scheduleEventsDispatch = NO;
162+
if (!_eventsDispatchScheduled) {
163+
_eventsDispatchScheduled = YES;
164+
scheduleEventsDispatch = YES;
165+
}
162166

163-
if (scheduleEventsDispatch) {
164-
[_bridge dispatchBlock:^{
165-
[self flushEventsQueue];
166-
} queue:RCTJSThread];
167-
}
168-
} else {
167+
// We have to release the lock before dispatching block with events,
168+
// since dispatchBlock: can be executed synchronously on the same queue.
169+
// (This is happening when chrome debugging is turned on.)
170+
[_eventQueueLock unlock];
171+
172+
if (scheduleEventsDispatch) {
169173
[_bridge dispatchBlock:^{
170-
[self dispatchEvent:event];
174+
[self flushEventsQueue];
171175
} queue:RCTJSThread];
172176
}
173177
}

0 commit comments

Comments
 (0)