Skip to content

Commit

Permalink
Remove usage of the ... operator in BackHandler
Browse files Browse the repository at this point in the history
This usage currently causes BackHandler to silently become non-functional when a Symbol polyfill is used.

Why?
- The [...obj] operator implicitly calls the object's iterator to fill the array
- react-native's internal Set polyfill has a load order issue that breaks it whenever a project uses a Symbol polyfill
- This means that when BackHandler tries to [...set] a Set of subscriptions the result is always an empty array instead of an array of subscriptions

Additionally it's worth noting that the current code is also wastefully inefficient as in order to reverse iterate over subscriptions it:
- Clones the Set (which fills it by implicitly running the set's iterator)
- Uses [...set] to convert the cloned set into an array (which also implicitly runs the iterator on the clone)
- Finally reverses the order of the array

This code fixes this problem by replacing the use of multiple Set instance iterators with use of `set.values()` which does not require the set's implicit iterator.

Fixes facebook#11968
  • Loading branch information
dantman committed Oct 20, 2017
1 parent 7e0b7ef commit bab01d3
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
4 changes: 2 additions & 2 deletions Libraries/Utilities/BackHandler.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ type BackPressEventName = $Enum<{
var _backPressSubscriptions = new Set();

RCTDeviceEventEmitter.addListener(DEVICE_BACK_EVENT, function() {
var backPressSubscriptions = new Set(_backPressSubscriptions);
var invokeDefault = true;
var subscriptions = [...backPressSubscriptions].reverse();
var subscriptions = Array.from(_backPressSubscriptions.values()).reverse();

for (var i = 0; i < subscriptions.length; ++i) {
if (subscriptions[i]()) {
invokeDefault = false;
Expand Down
4 changes: 2 additions & 2 deletions Libraries/Utilities/BackHandler.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ if (Platform.isTVOS) {

_tvEventHandler.enable(this, function(cmp, evt) {
if (evt && evt.eventType === 'menu') {
var backPressSubscriptions = new Set(_backPressSubscriptions);
var invokeDefault = true;
var subscriptions = [...backPressSubscriptions].reverse();
var subscriptions = Array.from(_backPressSubscriptions.values()).reverse();

for (var i = 0; i < subscriptions.length; ++i) {
if (subscriptions[i]()) {
invokeDefault = false;
Expand Down

0 comments on commit bab01d3

Please sign in to comment.