Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Incompatabile with nodejs 6.9.1: makeZoneAwareRemoveAllListeners addes 'undefined' arguments to real removeAllListeners calls #518

Closed
KrauseStefan opened this issue Nov 24, 2016 · 4 comments

Comments

@KrauseStefan
Copy link

zone.js wrappes the removeAllListeners call from node.js but breaks compatibility with the real version.

This happens because zone.js parsed undefined arguments to the real call when none should be.
This causes the line highlighted below to fail since node.js tests on arguments.length

link to the faulting line in utils.ts:
https://github.com/angular/zone.js/blob/master/lib/common/utils.ts#L293

snippet form events.js in node 6.9.1

// ...
EventEmitter.prototype.removeAllListeners =
    function removeAllListeners(type) {
      var listeners, events;

      events = this._events;
      if (!events)
        return this;

      // not listening for removeListener, no need to emit
      if (!events.removeListener) {
        if (arguments.length === 0) { // <--- length will always be 2, since undefined is always passed
          this._events = new EventHandlers();
          this._eventsCount = 0;
        } else if (events[type]) {
          if (--this._eventsCount === 0)
            this._events = new EventHandlers();
          else
            delete events[type];
        }
        return this;
      }
// ...

The function is used by xml2js to remove all event listeners by calling:
removeAllListeners() without arguments

@JiaLiPassion
Copy link
Collaborator

JiaLiPassion commented Nov 24, 2016

Thank you for finding the bug, I made a PR #516 to fix this issue.

@KrauseStefan
Copy link
Author

No problem, I guess we are quite a few people who needs this to work properly in a node.js env, I'll have a look through your changes

@JiaLiPassion
Copy link
Collaborator

It has been fixed, you can use the newest version to test.

@KrauseStefan
Copy link
Author

I finally got around to verify this one, yes this issue is gone, but I have a new one now. Will create a new issue for this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants