Broken BackAndroid handling

[rafales]

We use GitHub Issues for bugs.

Description

Handling back button with BackAndroid does not work - while adding handlers works fine they are never called. The reason behind this - Array.from(new Set([1, 2, 3])) returns empty array. Sets are used by the BackAndroid module.

Reproduction

• add new handler in BackAndroid and observe how it's not executed.

Additional Information

• React Native version: 0.36, .40
• Platform: Android
• Operating System: MacOS
• Device: Nexus 5, Android 6.0.1, patch lvl: October 5, 2016

[hramos]

Do you have any code samples that show how the error can be reproduced?

[dantman]

I have this same issue in RN 0.43.4.

I can confirm that:

• The BackAndroid.android.js I see contains var subscriptions = [...backPressSubscriptions].reverse();
• In the compiled version turns this becomes var subscriptions = [].concat(babelHelpers.toConsumableArray(backPressSubscriptions)).reverse();
• toConsumableArray uses Array.from(arr) for non-arrays
• When I dump a log for the expression Array.from(new Set([1, 2, 3])) into my project I get [] but in node.js when I do that I get [ 1, 2, 3 ].

[dantman]

I believe I have tracked down the underlying issue.

At the start of my code I import 'core-js/es6/symbol'; so that ES6 Symbol is available.

• React Native includes its own Set polyfill in Libraries/vendor/core/Set.js
• _shouldPolyfillES6Collection('Set') returns true so RN's Set polyfill is used
• When the polyfill is initialized toIterator.ITERATOR_SYMBOL is '@@iterator' because Symbol does not exist
• All of this happens before any application code runs or polyfills are required
• During runtime if you require the symbol polyfill or any polyfill that requires the symbol polyfill
  • Symbol actually exists and Symbol.iterator is defined
  • The Array.from polyfill will then look for iterators defined using the value of Symbol.iterator instead of '@@iterator'
  • Because RN's custom Set polyfill doesn't use the correct iterator symbol Array.from(new Set(...)) now always returns []

If I remove all code that leads to a Symbol polyfill being required, the bug goes away.

---

Workaround 1

Remove anything that depends on a Symbol polyfill (not a very reasonable workaround).

Workaround 2

After requiring something that uses a Symbol polyfill, use this to hack the correct iterator into RN's custom polyfill:

{
	// https://github.com/facebook/react-native/issues/11968
	// RN currently has a custom `Set` polyfill that is broken when you use an actual Symbol polyfill
	let toIterator = require('react-native/Libraries/vendor/core/toIterator');
	if ( toIterator.ITERATOR_SYMBOL !== Symbol.iterator && Set.prototype[toIterator.ITERATOR_SYMBOL] ) {
		Set.prototype[Symbol.iterator] = Set.prototype[toIterator.ITERATOR_SYMBOL];
	}
}

Short-term fix

BackAndroid (now BackHandler.android) doesn't actually have any reason to depend on fancy iterator behaviours. The line immediately after the ES6 [...arr] syntax is an ES3 style for(var i ...) loop. And the code is unnecessarily adding more low level loops (new Set(_backPressSubscriptions) clones the set, [...backPressSubscriptions] creates an array and uses an interator function to fill it with the values in the cloned set, .reverse() then reverses the order of the array).

A simple fix would be to replace this:

  var backPressSubscriptions = new Set(_backPressSubscriptions);
  var invokeDefault = true;
  var subscriptions = [...backPressSubscriptions].reverse();

with this:

  var invokeDefault = true;
  var subscriptions = [];
  _backPressSubscriptions.forEach(function(subscription) {
    subscriptions.unshift(subscription);
  });

(If you think unshift is sub-optimal I could equally use push (or pre-calculate the array size) and make the for loop run backwards)

Long-term fix

Long term this is going to be a general problem with this pattern of React Native hardcoding its own embedded polyfill and likely will result in other parts of React Native breaking – the short-term fix after all doesn't actually fix Set; Set, Map, any any other polyfill in RN implementing an iterator will still be broken.

React Native should either abandon low-level ES6 polyfills with conflict potential like Set and Map tweaking internal RN code so it does not rely on them or React Native should directly require core-js' set and map polyfills.

[dslounge]

haha I can't believe I just spent an entire day trying to figure out why the back button wasn't working. It's the issue @dantman described. I was importing the Symbol polyfill from core-js. Back button was working on chrome debug mode, not in the regular app.

[edgesite]

@dantman
Can you submit a pull request for the short-term fix since the long-term fix needs time to approach?

[hramos]

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

• Does the issue still reproduce on the latest release candidate? Post a comment with the version you tested.
• If so, is there any information missing from the bug report? Post a comment with all the information required by the issue template.
• Is there a pull request that addresses this issue? Post a comment with the PR number so we can follow up.

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.

[dantman]

I've added a PR

[cosmith]

For anyone still having this issue, since the commit 165b38a didn't actually fix it, you can use the fix by dantman by creating a new file where BackHandler doesn't use [...] or Set.values:

backhandler-custom.js

/**
 * Copyright (c) 2015-present, Facebook, Inc.
 * All rights reserved.
 *
 * This source code is licensed under the BSD-style license found in the
 * LICENSE file in the root directory of this source tree. An additional grant
 * of patent rights can be found in the PATENTS file in the same directory.
 *
 */

"use strict";

var DeviceEventManager = require("NativeModules").DeviceEventManager;
var RCTDeviceEventEmitter = require("RCTDeviceEventEmitter");

var DEVICE_BACK_EVENT = "hardwareBackPress";

type BackPressEventName = $Enum<{
    backPress: string,
}>;

var _backPressSubscriptions = new Set();

RCTDeviceEventEmitter.removeAllListeners(DEVICE_BACK_EVENT);

RCTDeviceEventEmitter.addListener(DEVICE_BACK_EVENT, function() {
    var invokeDefault = true;
    var subscriptions = [];
    _backPressSubscriptions.forEach(sub => subscriptions.unshift(sub));

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

    if (invokeDefault) {
        BackHandler.exitApp();
    }
});

/**
 * Detect hardware button presses for back navigation.
 *
 * Android: Detect hardware back button presses, and programmatically invoke the default back button
 * functionality to exit the app if there are no listeners or if none of the listeners return true.
 *
 * tvOS: Detect presses of the menu button on the TV remote.  (Still to be implemented:
 * programmatically disable menu button handling
 * functionality to exit the app if there are no listeners or if none of the listeners return true.)
 *
 * iOS: Not applicable.
 *
 * The event subscriptions are called in reverse order (i.e. last registered subscription first),
 * and if one subscription returns true then subscriptions registered earlier will not be called.
 *
 * Example:
 *
 * ```javascript
 * BackHandler.addEventListener('hardwareBackPress', function() {
 *  // this.onMainScreen and this.goBack are just examples, you need to use your own implementation here
 *  // Typically you would use the navigator here to go to the last state.
 *
 *  if (!this.onMainScreen()) {
 *    this.goBack();
 *    return true;
 *  }
 *  return false;
 * });
 * ```
 */
var BackHandler = {
    exitApp: function() {
        DeviceEventManager.invokeDefaultBackPressHandler();
    },

    /**
   * Adds an event handler. Supported events:
   *
   * - `hardwareBackPress`: Fires when the Android hardware back button is pressed or when the
   * tvOS menu button is pressed.
   */
    addEventListener: function(
        eventName: BackPressEventName,
        handler: Function
    ): {remove: () => void} {
        _backPressSubscriptions.add(handler);
        return {
            remove: () => BackHandler.removeEventListener(eventName, handler),
        };
    },

    /**
   * Removes the event handler.
   */
    removeEventListener: function(eventName: BackPressEventName, handler: Function): void {
        _backPressSubscriptions.delete(handler);
    },
};

module.exports = BackHandler;

And then import BackHandler from "./backhandler-custom"; and everything should work.

[AuroreM]

Thanks, I've used this custom BackHandler =)

Nevertheless I needed to import it with a new name :
import BackAndroid from "./backhandler-custom";
Otherwise the one from react-native where still called

[rohitgoyal]

@cosmith Tried this. It still doesn't work for me.
Created backhandler-custom file and imported in my App.js

In my App.js added event listener for hardwareBackPress like this

...
import BackHandler from "./backhandler-custom"
componentDidMount(){
    BackHandler.addEventListener('hardwareBackPress', this.handleBackButton);
    ....
}

componentWillUnmount(){
    BackHandler.removeEventListener('hardwareBackPress', this.handleBackButton);
    ....
}
handleBackButton() {
    console.log('back button pressed')
    console.log('Actions.state',Actions.state)
    // return true;
    if (Actions.state.index === 0) {
      console.log('index 0')
      BackHandler.exitApp();
      return false
    }
    console.log('index ',Actions.state.index)
    Actions.pop()
    return true
  }

[cosmith]

@rohitgoyal maybe try importing it with another name like AuroreM