Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[emitter] Fix sparse array handling in EventEmitter#listeners() #24546

Closed
wants to merge 1 commit into from

Conversation

ide
Copy link
Contributor

@ide ide commented Apr 21, 2019

Summary

Fixes a regression in 1f8b46a. The internal subscription vendor uses a sparse array to track listeners, which makes listener removal fast. When querying listeners, the sparse entries need to be removed. Array#filter is a built-in way to do this -> linked to the JS spec, which explains this.

Changelog

[General] [Fixed] - Fixed sparse array handling in EventEmitter#listeners()

Test Plan

Ran tests outside of this repo that now pass

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Expo Partner: Expo Partner labels Apr 21, 2019
Fixes a regression in facebook@1f8b46a#diff-141db70a3fb9a501015e4cb2508e3a65. The internal subscription vendor uses a sparse array to track listeners, which makes listener removal fast. When querying listeners, the sparse entries need to be removed. `Array#filter` is a built-in way to do this -> linked to the JS spec, which explains this.
Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

Ah, thanks for catching that! We didn't know why that filter was there and thought it was safe to remove, thanks for adding this comment.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @ide in b76acd3.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Apr 23, 2019
grabbou pushed a commit that referenced this pull request May 6, 2019
Summary:
Fixes a regression in 1f8b46a. The internal subscription vendor uses a sparse array to track listeners, which makes listener removal fast. When querying listeners, the sparse entries need to be removed. `Array#filter` is a built-in way to do this -> linked to the JS spec, which explains this.

[General] [Fixed] - Fixed sparse array handling in `EventEmitter#listeners()`
Pull Request resolved: #24546

Differential Revision: D15044790

Pulled By: cpojer

fbshipit-source-id: 0f1301618739357b4a0f5378b9584efe74f0f09a
@ide ide deleted the fix-emitter branch April 25, 2020 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Expo Partner: Expo Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants