Uploaded image for project: 'Alloy'
  1. Alloy
  2. ALOY-1607

Latest underscore version is breaking removeListener method in BaseController.js

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: CLI Release 7.0.3
    • Component/s: Tooling
    • Labels:
      None
    • Story Points:
      1
    • Sprint:
      2018 Sprint 05 Tooling

      Description

      Latest underscore version (1.8.3) used in Alloy 1.11.0 causes an error using $.removeListener method.

      https://github.com/appcelerator/alloy/blob/2a25b1d50fe22d3f5c4f423a6027e1c1d7748b88/Alloy/lib/alloy/controllers/BaseController.js#L496

      I don't know why Appc Team use underscore inside BaseController.js instead use built in prototype functions like 'forEach'. In fact bump to latest underscore version is breaking the normal usage of `$.removeListener`method and this is why:

      BaseController.js

       
      removeListener: function(proxy, type, callback) {
        _.each(this.__events, function(event, index) {
          if ((!proxy || proxy.id === event.id) &&
            (!type || type === event.type) &&
            (!callback || callback === event.handler)) {
            event.view.removeEventListener(event.type, event.handler);
       
            //This line is removing a item from __events array but a undefined value is placed in the removed index
            //self.__events.splice(index, 1); removes the item with no undefined value instead
            delete self.__events[index];
          }
        });
        return this;
      }
      

      What happens in the underscore 1.8.3 version _.each method?
      Well, it iterates in the array using a for, `undefined` values included.
      If event is undefined when event.view.removeEventListener(event.type, event.handler); it fails because view isn't in event

      [ERROR] Error catched: Cannot read property 'view' of undefined

      _.each = _.forEach = function(obj, iteratee, context) {
        iteratee = optimizeCb(iteratee, context);
        var i, length;
        if (isArrayLike(obj)) {
          for (i = 0, length = obj.length; i < length; i++) {
            iteratee(obj[i], i, obj);
          }
        } else {
          var keys = _.keys(obj);
          for (i = 0, length = keys.length; i < length; i++) {
            iteratee(obj[keys[i]], keys[i], obj);
          }
        }
        return obj;
      };
      

      What happens in the oldest underscore 1.6.0 version _.each method?
      Well, it checks if forEach method is present in `Array.prototype` and uses it. This method skipes `undefined`values.

      var ArrayProto = Array.prototype;
      var nativeForEach      = ArrayProto.forEach;
       
      var each = _.each = _.forEach = function(obj, iterator, context) {
          if (obj == null) return obj;
          if (nativeForEach && obj.forEach === nativeForEach) {
            obj.forEach(iterator, context);
          } else if (obj.length === +obj.length) {
            for (var i = 0, length = obj.length; i < length; i++) {
              if (iterator.call(context, obj[i], i, obj) === breaker) return;
            }
          } else {
            var keys = _.keys(obj);
            for (var i = 0, length = keys.length; i < length; i++) {
              if (iterator.call(context, obj[keys[i]], keys[i], obj) === breaker) return;
            }
          }
          return obj;
        };
      

      Use case in Titanium:

          $.addListener($.win, 'open', disableClick);
          $.addListener($.win, 'close', clean);
          $.addListener($.disable, 'click', disableClick);
          $.addListener($.label, 'click', close);
       
      function disableClick() {
          //Removes listener but undefined value is placed
          $.removeListener($.disable, 'click', disableClick);
        }
       
        function close(e) {
          Ti.API.info('Closing window');
          $.win.close();
        }
       
        function clean(e) {
          Ti.API.info('Cleaning window');
          try {
            Ti.API.info('Call $.removeListener()');
            $.removeListener();
          } catch (err) {
            //It tries to remove event to undefined value
            //[ERROR] Error catched: Cannot read property 'view' of undefined
            Ti.API.error('Error catched: ' + err.message);
          }
        }
      

      How to solve:

      How you can solve this? So easy...

      First way:

      Change _.each for forEach

      removeListener: function(proxy, type, callback) {
        this.__events.forEach(function(event, index) {
          if ((!proxy || proxy.id === event.id) &&
            (!type || type === event.type) &&
            (!callback || callback === event.handler)) {
            event.view.removeEventListener(event.type, event.handler);
            delete self.__events[index];
          }
        });
        return this;
      }
      

      Second way:

      Adding a simple check in the if...

      removeListener: function(proxy, type, callback) {
        _.each(this.__events, function(event, index) {
          if (event && (!proxy || proxy.id === event.id) &&
            (!type || type === event.type) &&
            (!callback || callback === event.handler)) {
            event.view.removeEventListener(event.type, event.handler);
            delete self.__events[index];
          }
        });
        return this;
      }
      

      As Konstantin Bueschel commented a way to avoid this undefined holes inside arrays is using splice instead delete

      self.__events.splice(index, 1);

        Attachments

          Activity

            People

            • Assignee:
              fmiao Feon Sua Xin Miao
              Reporter:
              jormagar Jorge Macias Garcia
            • Watchers:
              14 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Backbone Issue Sync

                • Backbone Issue Sync is enabled for your project, but we do not have any synchronization info for this issue.

                  Git Source Code