Issue with zone.js and non angular dom element events

Question

I'm facing an issue with Zone.js and non-angular DOM element events.

This happens when we assign a javascript function to any DOM element in the HTML mark up and then programmatically re-assign the same DOM element some other or the same function.

e.g. You have this mark up

<button id="button1" onclick="test1()">button 1</button>

and then somewhere in JS you re-assign button1 onclick with some other function

 document.getElementById('button1').onclick = test3;

In such scenarios, two function calls are made. First one is original assigned method and then the second one is the newly assigned method.

I’ve created a PLUNKER to demo this behavior.

In the example, we have an angular app and 2 non angular buttons on the screen. Each button is initially assigned a function which just prints some text to console.

One button (Button 1 on the onclick event ) just prints test 1 method called.

The other button (Button 2 on the onclick event) prints test 2 method called. And also assigns onclick of button1 a new function. This new function prints test 3 method called.

The scenario-

Let the application load. When you can see App works do the following:-
1. Open the debugger console.
2. Click button 2
3. Check the console
4. Click button 1.
5. Check the console.

The expectation.
• On click on Button 2 , we should see test 2 method called.
• On click on Button 1 , we should see test 3 method called.

The actual
• On click on Button 2, we see test 2 method called.
• On click on Button 1, we see test 1 method called. Followed by test 3
method called.

The PLUNKER link

https://plnkr.co/edit/ymKcHjWPrDiG73NfWBle?p=preview

On debugging , I see that one method is called DOM element event (test1 from onclick) and the other method (test3) is called by zonejs
(To this debugging, I had added console.log in zone.js file on my local machine)

I see that zonejs registers with allmost all events related to any DOM element on the screen.

So, is this an issue I'm facing ? or is it the expected behavior or am I going wrong some where? Please help with some info / pointers.

The Plunker doesn't seem to do anything. Might be a (temporary) Plunker issue though. — Günter Zöchbauer, Jul 19 '17 at 13:23
zone.js only affects event bindings that are made from within a zone. Angular runs within such a zone. If you do it outside, zone is not involved. — Günter Zöchbauer, Jul 19 '17 at 13:24
@GünterZöchbauer, `NgZone` is not involved. But the root zone still listens to all events — Max Koretskyi, Jul 19 '17 at 15:21
@GünterZöchbauer, I meant it patches and intercepts all async events. It's not related to Angular — Max Koretskyi, Jul 19 '17 at 15:27
@Maximus it does that for code that runs inside a zone only, at least AFAIK. — Günter Zöchbauer, Jul 19 '17 at 15:28
I'll try to show it on plunker, but did you'll see that console prints log of previous as well as the currently assigned onclick function? How would one explain that? Its very confusing. Even I feel that zone shouldnt be involved... but my debugging says that , one of the method called is a wrapFn (the wrapper function) zone creates over all its events... — Bhavik Shah, Jul 19 '17 at 16:04
@GünterZöchbauer, see [this plunker](https://plnkr.co/edit/3ceW3e1BsjbHRDPQhvDH?p=preview). I haven't created a child zone. Only the root zone is available and it listens for everything — Max Koretskyi, Jul 19 '17 at 16:18
@BhavikShah, make a plunker, without it it's impossible to debug — Max Koretskyi, Jul 19 '17 at 16:18
https://plnkr.co/edit/jcAxzmivK7h2RBx7fA2b?p=preview try this one @Maximus . See if it helps.. in the console you'll see zone getting called... — Bhavik Shah, Jul 19 '17 at 16:23
root zone listening to every event isn't a problem, but its triggering extra events.. (I don't know if thats the expected behaviour though) — Bhavik Shah, Jul 19 '17 at 16:29
@BhavikShah, see [my answer](https://stackoverflow.com/a/45197095/2545680), it'll explain what happens — Max Koretskyi, Jul 19 '17 at 17:18

Max Koretskyi · Accepted Answer · 2017-07-19T17:24:44.423

The problem is that zone patches onclick and uses addEventListener/removeEventListener to register/remove a callback for the button. But the onclick registers its own independent handler that can't be removed with removeEventListener. So effectively you end up with two handlers for the click event:

function patchProperty(obj, prop, prototype) {
    var desc = Object.getOwnPropertyDescriptor(obj, prop);
    ...
    desc.set = function (newValue) {
        ...
        var previousValue = target[_prop];
        if (previousValue) {
            // !!!!!! ------------------ it doesn't remove onclick here ---------- !!!!!!
            target.removeEventListener(eventName, previousValue); 
        }
        if (typeof newValue === 'function') {
            var wrapFn = function (event) {
              console.log(' zone wrap function called for--\n');
              console.log('targetID '+target.id);
              console.log('event is '+eventName);
                var result = newValue.apply(this, arguments);
                if (result != undefined && !result) {
                    event.preventDefault();
                }
                return result;
            };
            target[_prop] = wrapFn;

            // !!!!!! ------------------ but adds new handler here ---------- !!!!!!
            target.addEventListener(eventName, wrapFn, false);

This situation can be easily reproduced without zones:

    function onClickHandler() {
        console.log('onClickHandler');
    }

    document.addEventListener("DOMContentLoaded", function() {
        const button = document.querySelector('button');
        button.removeEventListener('click', button.onclick);
        button.addEventListener('click', ()=> {
            console.log('addEventListener')
        });
    });

   <button onclick="onClickHandler()">Hello</button>

Yes..Got it. Thanks :) and so it correct to say that zone.js has an issue ?? — Bhavik Shah, Jul 20 '17 at 05:09
It's hard to tell. On the surface, yes, it does seems that `zone.js` doesn't remove handler correctly, but I don't know there thinking. Create a github issue and we will see. If you do, please put a reference here. — Max Koretskyi, Jul 20 '17 at 05:11
well, we went on checking the zone.js code and the code itself has a reference to the issue. https://github.com/angular/zone.js/issues/525 — Bhavik Shah, Jul 20 '17 at 07:19
I added it to my previous comment . Here, again, https://github.com/angular/zone.js/issues/525 — Bhavik Shah, Jul 20 '17 at 07:21
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/149725/discussion-between-bhavik-shah-and-maximus). — Bhavik Shah, Jul 20 '17 at 13:52

score 2 · Answer 2 · answered Jul 20 '17 at 10:08

One of the resolution is to patch "desc.set" method of zone.js to resolve this issue.

function patchProperty(obj, prop, prototype) {
var desc = Object.getOwnPropertyDescriptor(obj, prop);
...
desc.set = function (newValue) {
...
var previousValue = target[_prop];
if (previousValue) {
    target.removeEventListener(eventName, previousValue);
}//Start Fix
else{ 
    if(originalDescGet){
         if (typeof target['removeAttribute'] === 'function') {
             target.removeAttribute(prop);
         }
    }   
}//End fix

This solution seems to work. Thanks :) I'll update the GitHub with this issue and put the link here. — Bhavik Shah, Jul 21 '17 at 09:02

score 1 · Answer 3 · answered Jul 19 '17 at 17:16

1

Not angular but zonejs is causing this behavior. Working with some trial error.

https://plnkr.co/edit/cnDy4utsWYbhaXIBluNw?p=preview

Moved inline events listeners to window.onload
updated test2() method to remove and add listener for button1

answered Jul 19 '17 at 17:16

Avi

105
3
10

yup.. in my question's comment we had the same discussion. So in legacy systems, this creates an issue... have u faced such issues? how to overcome them? – Bhavik Shah Jul 20 '17 at 05:10

Issue with zone.js and non angular dom element events

3 Answers3