Skip to content

Commit

Permalink
Fix - argument type in RCTEventEmitter
Browse files Browse the repository at this point in the history
Reviewed By: javache

Differential Revision: D6528139

fbshipit-source-id: 170c2359bcc67131330d091e3707124018053938
  • Loading branch information
Rahul Ramachandran authored and facebook-github-bot committed Dec 13, 2017
1 parent fbf0aed commit eaa8499
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
2 changes: 1 addition & 1 deletion React/Modules/RCTEventEmitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@
- (void)stopObserving;

- (void)addListener:(NSString *)eventName;
- (void)removeListeners:(NSInteger)count;
- (void)removeListeners:(double)count;

@end
7 changes: 4 additions & 3 deletions React/Modules/RCTEventEmitter.m
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,13 @@ - (void)dealloc
}
}

RCT_EXPORT_METHOD(removeListeners:(NSInteger)count)
RCT_EXPORT_METHOD(removeListeners:(double)count)
{
if (RCT_DEBUG && count > _listenerCount) {
int currentCount = (int)count;
if (RCT_DEBUG && currentCount > _listenerCount) {
RCTLogError(@"Attempted to remove more %@ listeners than added", [self class]);
}
_listenerCount = MAX(_listenerCount - count, 0);
_listenerCount = MAX(_listenerCount - currentCount, 0);
if (_listenerCount == 0) {
[self stopObserving];
}
Expand Down

2 comments on commit eaa8499

@vovkasm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without detailed motivation in commit message, this change looks very strange... even more, it looks dangerous... what if one pass 1e100 as argument, what to do with that? what exact number is? What if one pass 1.5? This should be error, but it will not...
Very bad 👎

@danielgindi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, WTF? How can there be a listener count of 1.5?
This commit does not make any sense.

Please sign in to comment.