Event bubbling

[mikemadest]

What is the issue?

Currently when dispatching an event to a node, that event won't bubble up to parents.
This is also used in linkedom which is the reason I started looking at it.

What could be done?

I pulled the code and started an update:

    define(proto, 'dispatchEvent', function (event) {
      var secret = wm.get(this);
      if (secret[event.type]) {
        var listeners = secret[event.type].slice(0);
        define(event, 'target', this);
        define(event, 'currentTarget', this);

        // if an event listener invoke stopImmediatePropagation it should stop calling other listeners
        for (var i = 0; i < listeners.length && !event._stopImmediatePropagationFlag; i++) {
          dispatch.call(event, listeners[i]);
        }
        delete event.currentTarget;
        delete event.target;
      }

      // should probably be done differently: https://dom.spec.whatwg.org/#event-path
      // but should not be too much of an issue for now
      if (event.bubbles && !event._stopPropagationFlag) {
        if (this.parentNode) {
          this.parentNode.dispatchEvent(event);
        }
      }
      return true;
    });

This is a very simple bubbling but it should works fine and could be improved in more PR eventually.
I created a branch, maybe I could push it and create a PR?
Thanks!

[WebReflection]

I understand the use case but EventTarget has no concept of a parentNode in general ... meaning, I am not sure we should fix this here, instead of fixing it in LinkeDOM

[mikemadest]

Fair point, but there is a concept of "get the parent": https://dom.spec.whatwg.org/#ref-for-get-the-parent

Maybe this could this be used? I see in linkedom there is a getter for parentElement but parentNode was great for that, and _getTheParent method could be added to EventTarget to match the specs.

It feels "expected" that the dispatching of the event would take care of the event bubbling and that it could be in EventTarget. Other codes do something similar (except they build the event path before triggering all listeners).

[WebReflection]

thing is ... this is kinda the TC39 standard answer to NodeJS emitter ... and emitter has no concept for parentNode, 'cause emitters are never a DOM related thing (except when they are, like JSDOM or LinkeDOM) ... all I am saying, is that I am not sure we should patch/fix this in here, 'cause DOM has nothing strictly to do with EventTarget, for what I understand ... or does it?

[mikemadest]

I'm confused as to why EventTarget would have nothing to do with DOM because the project description does have a link pointing to the DOM EventTarget spec and my proposition was based on that. Specs for EventTarget do mention a way to "get the parent" as I linked previously (through a method though, parentNode is indeed a bit too linkedom specific).
My apologies if I'm wrong I don't mean to waste your time or add useless features here.

I could look into linkedom code instead if you think that's more relevant.
Could it have its own version of EventTarget instead of importing this one? Or extend it?
Event related code is present there, adding bubbling abilities should make sense while not making the overall code much heavier.

As to why I'm doing that, in my company we are looking for alternative solutions to jsdom and I find linkedom concept very interesting. My goal was to see what I could do to use linkedom in unit tests and contribute if that works for you.

[WebReflection]

I'm confused as to why EventTarget would have nothing to do with DOM

in NodeJS a new EventTarget is a valid reference, and (AFAIK) it doesn't care about bubbling.

My apologies if I'm wrong I don't mean to waste your time or add useless features here.

it's a valid issue, imho, I'm just trying to understand where it fits the best. the parentNode is a very DOM specific thing, but this poly works in browsers like, as well as in NodeJS and other JS environments (gjs, etc), hence my "brainstorming".

Could it have its own version of EventTarget instead of importing this one? Or extend it?

extending was my thinking, yes

My goal was to see what I could do to use linkedom in unit tests and contribute if that works for you.

it does, so keep it coming, and please bear with me, it's just that @ungap has broader scope, even if it's likely used for browsers polyfills only, but it has things that work down to older NodeJS versions too, and I'd like to not break that.

[mikemadest]

extending was my thinking, yes

Perfect, that works for me and I get your point now 👍

I've been reading Node discussion about their implementation of EventTarget and it is expected to have null "get the parent" algorithm by default which is the case here (more on that: nodejs/node#33556 (comment)).
So extending seems like the way to go 👍

Thanks for your answers!

[mikemadest]

I started looking at extending EventTarget and putting the bubbling aside there are still some aspects of the implementation that are limiting:

1. listeners are private here and can't be read from any extending class
2. dispatchEvent should take into account the stop immediate propagation flag and stop the call of other listeners
   It may be hard to extend this without making any changes?

[WebReflection]

Thanks for looking into this. Here my answer:

1. that's a feature that I am not sure I should change
2. the stopImmediatePropagation is indeed something not implemented

I start thinking maybe it's worth rewriting this module from scratch for LinkeDOM only as things might be much simpler, as the target is NodeJS, not browsers / others.

[WebReflection]

P.S. in node there is also native EventTarget and there stopImmediatePropagation works out of the box ... not sure which version ships that though, but we might just extend it and hook ourselves into the dispatch method. However, I am not sure we want to implement the whole capture/bubbling phase, as that is a road to tons of extra changes for I am not even sure which gain.

[mikemadest]

we might just extend it and hook ourselves into the dispatch method. However, I am not sure we want to implement the whole capture/bubbling phase

I agree, looking at the code extending Node's EventTarget could work, I have to check it. About bubbling, I'd like to keep it simple unless there is a need for more. And if needs arise it can be improved later

[WebReflection]

Done a quick check, only v16 has EventTarget ... so I'd still use this module as base class, extend it as DOMEventTarget in Linkedom, and use that extend instead.

I'll fix the stopImmediatePropagation in here, but the rest needs to be implemented in LinkeDOM ... I don't have much time these days, so PRs welcome (at least the LinkeDOM one).