From 165b38ab7abdf9d769efddde39da35a03564284b Mon Sep 17 00:00:00 2001 From: Daniel Friesen Date: Fri, 20 Oct 2017 17:49:21 -0700 Subject: [PATCH] Remove usage of the ... operator in BackHandler Summary: This usage of `...` 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 a simple `.forEach` loop that unshifts directly into the final array. I've tested this by opening the repo's RNTester app on Android and tvOS ensuring that the back handler works before changes, the application crashes when I introduce an error (to verify my code changes are being applied to the app), the back handler works and after changes. Fixes #11968 Closes https://github.com/facebook/react-native/pull/15182 Differential Revision: D6114696 Pulled By: hramos fbshipit-source-id: 2eae127558124a394bb3572a6381a5985ebf9d64 --- Libraries/Utilities/BackHandler.android.js | 4 ++-- Libraries/Utilities/BackHandler.ios.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Libraries/Utilities/BackHandler.android.js b/Libraries/Utilities/BackHandler.android.js index d8ca9b5eb..21e320c5e 100644 --- a/Libraries/Utilities/BackHandler.android.js +++ b/Libraries/Utilities/BackHandler.android.js @@ -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; diff --git a/Libraries/Utilities/BackHandler.ios.js b/Libraries/Utilities/BackHandler.ios.js index 958ec69e5..7c9a00ecc 100644 --- a/Libraries/Utilities/BackHandler.ios.js +++ b/Libraries/Utilities/BackHandler.ios.js @@ -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;