Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow custom "get the parent" algorithms for EventTargets #583

Open
zandaqo opened this issue Mar 5, 2018 · 29 comments
Open

Allow custom "get the parent" algorithms for EventTargets #583

zandaqo opened this issue Mar 5, 2018 · 29 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: events

Comments

@zandaqo
Copy link

zandaqo commented Mar 5, 2018

In the future we could allow custom get the parent algorithms. Let us know if this would be useful for your programs. For now, all author-created EventTargets do not participate in a tree structure.

I'd like to see that future. That is, the ability to create a tree structure for author-created EventTargets like the one we have with Nodes, with bubbling and capturing, and so forth. Two points in support of this:

  1. It would just make the platform more consistent when all EventTargets behave similar or have ways to do so. Right now we have a "weird" limitation to keep in mind when we work with custom EventTargets.
  2. Listening just to a parent instead of each of its children simplifies our code when we work with Nodes, and same can apply to author-created EventTargets.

An example use case:
Let's take the classic Todo app example. Imagine that each todo item is a custom EventTarget emitting "change" events, and the whole list is an array. Now, if we need to respond to any change in the list, say, recalculate the number of items, we have to add event listeners to each item. However, if we could define our array as the parent for each item where events could bubble to (or be captured on) the parent, we could get away with just one listener on the array. Since the items and the array already have strong coupling, adding an extra "parent" pointer wouldn't make difference in terms of the design.

@annevk annevk added the needs implementer interest Moving the issue forward requires implementers to express interest label Mar 7, 2018
@annevk annevk changed the title Allow custom "get the parent" algorithms for EventTargets. Allow custom "get the parent" algorithms for EventTargets Mar 7, 2018
@annevk
Copy link
Member

annevk commented Sep 14, 2018

To what extent are you interested in also having shadow tree capabilities for your custom event targets? E.g., being able to hide event targets from composedPath() and such. (This came up in #684.)

@zandaqo
Copy link
Author

zandaqo commented Sep 18, 2018

@annevk I cannot think of a use case for shadow tree capabilities. If we are building tree structures out of these custom EventTargets solely to propagate events, having a capability of hiding/skipping certain nodes seems superfluous--one can just avoid putting those hidden nodes into the said tree structure.

@annevk annevk added the addition/proposal New features or enhancements label Apr 11, 2019
@aayushch
Copy link

We are building web extensions which would like to implement events, to propagate critical occurrences from the browser api event listeners back to the extension business logic layer.

We would want to leverage the EventTarget implementation to build propagation, rather doing it in an ad-hoc way.

The ability to bubble the events is going to be a huge advantage given we can override/implement a custom 'get the parent' algorithm.
The shadow tree capabilities are not really necessary for author created event targets.

@clshortfuse
Copy link

clshortfuse commented Apr 25, 2020

I'm currently using a polyfill for EventTarget in NodeJS environments. I see Deno will support EventTarget out of the box, so at least we're going to get nice usage from EventTarget.

But, for my polyfill, I'm creating a custom "get the parent" implementation via WeakMap as to not pollute the the property list.

/**
 * @typedef {Object} EventDispatcherEventTargetPrivateFields
 * @prop {Map<string, Listener[]>} listeners
 * @prop {function():EventDispatcherEventTarget} getParent
 */

/** @type {WeakMap<any, EventDispatcherEventTargetPrivateFields>} */
export const privateEventTargetFields = new WeakMap();

export default class EventDispatcherEventTarget {
  constructor() {
    privateEventTargetFields.set(this, {
      listeners: new Map(),
      getParent: () => null,
    });
  }
  
  /* ... */

getParent() gets checked and called during dispatchEvent(), much like how browsers do it. The problem is my custom implementations has to know to set privateEventTargetFields.get(eventTarget).getParent = callback;

The reason is I'm using a context => listener => client configuration for socket connections. Client connections themselves emit events which can be captured by the listener (TCP or WebSocket). Propagation can be stopped or it can continue bubble to the context (service that spawns the listeners).

It makes sense because the application architecture is tree-like already. Unfortunately, if we decide to port to Deno we will not be able to the official EventTarget implementation because spec has no "get the parent" function and no standard as to how to pass the "get the parent" function in the constructor.

Edit: I just want to add that this continues to make more sense considering we can do new EventTarget() now which I use for custom events that has nothing to do with the DOM. Also, some objects (eg: service workers, window, navigator) extend EventTarget but have little to no relation to the DOM (and workers can't even access the DOM), so I would say Events has branched outside the basic DOM scope already.

@jchayan
Copy link

jchayan commented Apr 5, 2021

+1 for this... In my case, it's just a whim. I'm currently writing a tiny widget's library where widgets are tree nodes (they have a parent and children), so I wanted to extend EventTarget and add event bubbling support, I stumbled upon this because I tried overriding dispatchEvent, so that:

class ElementNode extends EventTarget {
  constructor () {
    super();
    this.parent   = null;
    this.children = [ ];
  }

  append (nodes) {...}
  remove (node) { ... }
  walk (visitor) { ... }
  dispatchEvent (event) {
    super.dispatchEvent(event);
    if (event.bubbles && this.parent) {
      this.parent.dispatchEvent(event); // Losing event.target here :(
    }
  }
}

The original event.target target is lost across calls because I'm manually dispatching the event in the parent, which then sets the target to be the parent. After reading the spec (https://dom.spec.whatwg.org/#concept-event-dispatch) I realized that the get the parent function can't be overridden and besides I can't just reassign event.target as it's read-only (and I understand why)

If I were to think of the benefits I'd say that we would gain standardization... Allowing this customization can open the doors for other libraries or frameworks to rely on the EventTarget and CustomEvent APIs instead of everyone rolling their own "event emitter" solutions.

@clshortfuse
Copy link

clshortfuse commented Apr 6, 2021

Deno actually uses a somewhat custom method of "get the parent" that allows custom overloading. The check occurs by checking if the EventTarget "is a node" by checking if has a nodeType property. Then it uses .parentNode. That means an Javascript Object that has nodeType can potentially return a parent (via .parentNode).

https://github.com/denoland/deno/blob/704e1e53307130012da974f42d2cad94e07b5664/op_crates/web/02_event.js#L438

This is piggybacking off Node, but to write a proper shim for it (I've tried it), you have to implement Node, ChildNode, ParentNode, Document. and some others that my escape me at the moment. That would be a lot of work for user agents to include and force them to require support something far more complicated just EventTarget. The Deno team is using something custom as is for lack of a standard, and I can totally understand doing it like that instead of writing all of Document.

I can only realistically conclude that the EventTarget may be constructed with a eventTargetInit object that may have a get the parent value. So you can maybe do something like new EventTarget({ getParent: myMasterParentFn }), for example. It keeps it strictly within EventTarget without telling user agents to support more complex types, or having developers need to add on something heavy like JSDom.

@rniwa
Copy link
Collaborator

rniwa commented Apr 6, 2021

Hm... we don't want to run arbitrary scripts just to get the parent. It needs to be some kind of setter/getter that UA would implement on EventTarget interface.

@jchayan
Copy link

jchayan commented Apr 8, 2021

@rniwa Do you think that having that setter/getter is a simple thing to do? And if so, what determines if that should be added to the spec?

To be honest, I'm just curious

@rniwa
Copy link
Collaborator

rniwa commented Apr 8, 2021

@rniwa Do you think that having that setter/getter is a simple thing to do? And if so, what determines if that should be added to the spec?

Basically, you need to get at least two implementors interested in your proposal, write a DOM/HTML spec PR and WPT tests.

@clshortfuse
Copy link

clshortfuse commented Apr 9, 2021

My concerns with a getter/setter is a hard reference could leave an object in memory, prohibiting it from being garbage collection. With DOM nodes, once a child is detached from its parent, .parentNode becomes null (or undefined, not sure). The parent can then be garbage collected if it's no longer referenced. That's one of the reasons why I leaned to a function.

The other idea I had with my custom implementation is a EventTarget parent registry. Basically, instead of the child explicitly stating what it's parent is, a global registry can be used. eg: parent = EventTargetParentRegistry.get(child). Here, a user agent can basically keep a dictionary of sorts since no child can have multiple parents.

A pure JavaScript solution (polyfill) could look something like this:

class EventTargetParentRegistry {
  /** @type {WeakMap<EventTarget, WeakRef<EventTarget>>} */
  static #registry = new WeakMap();

  static get(child) {
    return this.#registry.get(child)?.deref();
  }

  static set(parent, child) {
    this.#registry.set(child, new WeakRef(parent));
  }

  static delete(child) {
    this.#registry.delete(child);
  }
}

During the get the parent phase, the user agent (or polyfill) would use EventTargetParentRegistry.get(child) for EventTargets that are not Node objects.

@rniwa
Copy link
Collaborator

rniwa commented Apr 9, 2021

My concerns with a getter/setter is a hard reference could leave an object in memory, prohibiting it from being garbage collection. With DOM nodes, once a child is detached from its parent, .parentNode becomes null (or undefined, not sure). The parent can then be garbage collected if it's no longer referenced. That's one of the reasons why I leaned to a function.

I'm not sure how that is consistent with the idea that EventTarget can have a parent. Is the idea that the event will propagate to the parent object only if that's still alive?? Literally nothing in the web platform works like that. I'm not certain we want to add API like this given its behavior will be highly dependent on a particular implementation of GC.

@keithamus
Copy link

keithamus commented Jan 25, 2023

In #1146 I suggested a method that could be available on EventTarget:

It would be great to introduce some kind of mechanism for EventTargets to associate with one another to allow for event bubbling and capturing. Perhaps EventTarget could have something like associateAncestor(ancestorEventTarget, { capture: true, bubble: true }) which could do the necessary internal wiring to make an event targets event capture & bubble upward to an ancestor? Existing EventTargets could throw when this is called, but it would allow for designs in userland to make use of a fairly large feature within EventTarget.

Having a method on the child event target allows it to perform necessary cleanup if called multiple times, and allows for customisation of the different phases. For example I can imagine some implementations opting out of the capturing phase and only allowing bubbling to occur.

I am not precious about the implementation, but I'm happy to put the work in provided implementer interest. @rniwa is your commentary on this issue an indication of interest? Perhaps @mfreed7 or @domenic could also provide some insight on to interest within Chrome?

@smaug----
Copy link
Collaborator

It would help implementers (at least Mozilla) if there were a bit more concrete proposal here.

It sounds like the use cases require effectively a DOM node tree, without the objects being actual nodes, but some other types of EventTargets. And one should be able to modify the tree using similar concepts as DOM tree.
And when dealing with mutations, the operations need to ensure one doesn't cause any cycles.

Minimal API from Event handling point of view would be something like setParentEventTarget(EventTarget? parent).
That should throw if 'this' is some other native EventTarget than EventTarget itself, or if parent is found in the parent chain or parent is 'this'. Calling setParentEventTarget(null) would be left to the user of the API - like the rest of the possible tree operations.
And event target path doesn't care about bubble or capture. The path is path, and how an event propagates through it depends on the properties of the event itself.

@keithamus
Copy link

An event only has a bubble flag which determines if it executes the bubbling & capturing phases. Capturing & bubbling phases are not both always desirable in a tree, so it would be useful to be able to turn one or the other off without huge workarounds.

@clshortfuse
Copy link

clshortfuse commented Jan 30, 2023

About two years later, I better understand @rniwa 's point about GC complexity.

A class that extends EventTarget can have a getter setter related to getting the parent. The Node (class) implementation can return Node.prototype.parentNode. That allows our (authors') extensions of EventTarget to handle the tree ourselves along with removal and concerns about GC (I'd probably use WeakRef internally). Implementers (Apple/FireFox coders) just have to invoke the getter and that's it.

If that's the right idea, let me know and I can write up a proposal draft. Something like

class MyCustomEventTarget extends EventTarget {

  /** @override */
  get parentEventTarget() {
    return AUTHOR_EVENTTARGET_WEAKMAP.get(this);
  }
}

IIRC, bubbling and capture are part of composedPath which shouldn't need any extra code other than a "get the parent" function/getter.

@annevk
Copy link
Member

annevk commented Feb 4, 2023

@keithamus events always perform the capturing phase. Making that optional at dispatch somehow would be a distinct feature request we should track separately.

@clshortfuse I don't think user agents should have to invoke JavaScript to traverse the tree. As @smaug---- suggests it should be possible for the user agent to have access to the entire path and impose constraints on it.

@LeaVerou
Copy link

Came here to file this exact same issue after naively trying to just set parentNode, hoping it may Just Work™ (narrator: It didn’t). It would be quite useful for authors to be able to reflect hierarchical relationships between data and have events propagate.

@keithamus
Copy link

keithamus commented Oct 9, 2023

How about overloading the constructor of an EventTarget to allow assignment of a parent EventTarget?

[Exposed=*]
interface EventTarget {
  constructor(parent?: EventTarget);
  
  // ...
}

If the parent is undefined or null then the EventTarget is considered to have no parent, otherwise assert that parent is EventTarget. For Node, ShadowRoot and Document, the constructor remains as-is and they continue to use their custom get the parent algos.

@annevk
Copy link
Member

annevk commented Oct 9, 2023

I think that is probably a workable API (no need for null though, optional parent argument or dictionary with optional parent member seems fine), but we will also need to add loop detection in https://dom.spec.whatwg.org/#concept-event-dispatch. Perhaps that should lead to an exception? I think you can only ever reach it from dispatchEvent() so that should be okay.

@keithamus
Copy link

keithamus commented Oct 9, 2023

Would something like c88f115 suffice? Should I raise this and work on some WPT/implementations?

@annevk
Copy link
Member

annevk commented Oct 9, 2023

That's a start, but it'll need some more work to formalize it properly. (In particular you probably want to define some kind of internal field that holds the parent and that "get the parent" ends up returning. And the IDL syntax you're looking for is optional EventTarget parent.)

@clshortfuse
Copy link

clshortfuse commented Oct 9, 2023

Do we want EventTargetInit in case we want more options in the future? Or would being an instance of EventTarget be enough to create an init object later down the road, if ever?

@keithamus Thanks for working on this.

I used init before and used a function instead, which is where we got to the garbage collection talk. We should be able to orphan and move those event targets, which means nulling and setting that value later during runtime. For my arbiter=>server=>client setup, orphaning is common.

@keithamus
Copy link

An init object could make sense. My understanding is that GC can dispose of the init object while retaining a reference to the parent, provided it is in the constructor. Getters can be retrieved during construction to capture the parent in an internal field.

@LeaVerou
Copy link

LeaVerou commented Oct 9, 2023

What about enabling parentNode to just work instead of adding more config? It should be fairly easy to investigate if this would be web compatible, but since bubbling is opt-in, I suspect it could be since there's no point in dispatching events that bubble on non-DOM nodes today, so even if objects have a parentNode property, it wouldn't do anything special.

This also means that we could even eventually port other DOM concepts to arbitrary objects with a tree-like structure (e.g. things like append() etc).

For objects that use another property to point to the parent object (e.g. parent), all this needs is

get parentNode() {
	return this.parent;
}

set parentNode (node) {
	this.parent = node;
}

Benefits:

  • Predictable and discoverable
  • Consistent across DOM and non-DOM objects
  • More flexibility for porting other DOM concepts in the future (see above)

Drawbacks:

  • Potential compat issue (needs research)
  • Less flexibility around object schema and parentNode is not a great name

Note that there is ample precedent around special property or method names doing special things rather than specifying properties in some init object, especally in Custom Elements.

Another idea to avoid additional config could be to insert another subclass between EventTarget and Node, e.g. AbstractNode, for objects that have parentNode and other tree methods, but are not DOM nodes.

That said, if any of this is entirely out of the question, it should definitely be an init object instead of a positional argument, see TAG principle 6.4 Accept optional and/or primitive arguments through dictionaries

@clshortfuse
Copy link

@LeaVerou What you suggest is how Deno already works. The code complexity around polyfilling Node is a quite a bit. I've tried it and ended up just wrapping EventTarget instead.

This is piggybacking off Node, but to write a proper shim for it (I've tried it), you have to implement Node, ChildNode, ParentNode, Document. and some others that my escape me at the moment. That would be a lot of work for user agents to include and force them to require support something far more complicated just EventTarget

#583 (comment)

Looking at my code, I have a bastardization of Node as a shim for the tree, without all the document stuff.

Subclassing between EventTarget and Node is probably better, though we'd have to define what goes in and what doesn't (eg: do we need to include insertBefore).

The only "concern" I can see about passing an EventTarget instead (new EventTarget(parentEventTarget)) of an init object is that EventTarget instances can be overloaded with their own properties, and I would want to avoid the possibilty of that custom EventTarget object later conflicting with an EventTargetInit property. For example:

class FooEventTarget extends EventTarget {
  bubbles = false;
}

Then if we make an EventTargetInit later with {bubbles: true, parent: foo} , it would be unclear if they're passing the parent or an EventTarget instance that has a property when doing new EventTarget(new FooEventTarget()).

@domenic
Copy link
Member

domenic commented Oct 10, 2023

Note that the design proposed here, of requiring an EventTarget parent object instead of a "function that returns an EventTarget" get-the-parent algorithm, means that this will only be usable when one is constructing one's tree of EventTargets in a top-down manner, where parents exist before children.

+1 to not conflating parentNode (part of the public Node interface) and "get the parent" (part of the EventTarget internal implementation).

@annevk
Copy link
Member

annevk commented Oct 10, 2023

@LeaVerou the way custom elements handle this is very specific though to avoid crossing the C++/JS bridge too many times. Having to run a lot of script while building up an event's path isn't acceptable.

It does seem like there are a number of use cases where you want to be able to manipulate the parent post-construction. That argues for some kind of revealing constructor pattern or something similar to ElementInternals, e.g.,

[Exposed=*]
interface EventTargetInternals {
  attribute EventTarget parent;
};

callback EventTargetCallback = undefined (EventTargetInternals internals);

partial interface EventTarget {
  constructor(optional EventTargetCallback cb);
};

@smaug----
Copy link
Collaborator

I don't understand the comments about mixing bubbles and EventTarget.
Bubbling is a property of the Event, not EventTarget (and it just tell that the event dispatch does only capturing and target phases).

And however we get the parent object, it needs to be very fast. Native implementation must not be required to touch any JS at that point (event dispatch is extremely performance critical).

@keithamus
Copy link

Tried to write up a spec around @annevk's IDL sketch of an EventTargetInternals: https://github.com/whatwg/dom/pull/1230/files. I'll noodle around with an implementation in the coming weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: events
Development

No branches or pull requests

10 participants