WICG / webcomponents

Web Components specifications
Other
4.36k stars 370 forks source link

Options for controlling specifics of "slotchange" events? #933

Open Westbrook opened 3 years ago

Westbrook commented 3 years ago

By default a <slot> observes its children with just about the least complex options of a Mutation Observer. Essentially,

new MutationObserver((mutationsList) => {
    for(const mutation of mutationsList) {
       mutation.target.dispatchEvent(new Event('slotchange'));
    }
}).observe(slotElement, { childList: true });

This means that a developer listening for the slotchange event can expect to get this event when direct child nodes are added or removed, but not when attributes on those nodes or text content of those nodes (when not elements) are changed. While this lower lever of complexity does well to ensure that a page full of <slot> elements is not ridiculously expensive to maintain, it does cause some surprise in developers leveraging this API for the first time (or for the 100th times as I find this really hard to actually keep in my brain). Once I am reminded of it, I often need to spin up my own Mutation Observer, anyways, which feels like it sort of defeats the perf benefit of keeping the slotchange trigger less complex in those cases. Here's an overly complex version of me doing just that: https://webcomponents.dev/edit/F4jBbQpeMSujg9FtRrtZ/src/index.ts

As this >100th time developer, and in the name of 1st time developers, I'd love to see a path towards being able to opt-in to more functionality here.

How could we supply our own options dict to that the above was more like the following?

new MutationObserver((mutationsList) => {
    for(const mutation of mutationsList) {
       mutation.target.dispatchEvent(new Event('slotchange'));
    }
}).observe(slotElement, developerSuppliedOptions || { childList: true });

I'm not sure if this actually uses a Mutation Observer under the covers or not (though at one point, I thought heard that it did), but I'd like to start a conversation around the implication that it should (if it's not) and the possibility of being able to customize the options dict sent to observer() in the case that it was. Below I'll outline a straw person for some options to being able to do so.

I look forward to your patient support in this endeavor as I've followed the process of conversations like this in the past, but this is the first that I can remember attempting to start.

All of my slots should act like this

In the case that a developer wanted all of their slots to act like this, it would see that adding this to the attachShadow() options dict would be a really useful idea.

constructor() {
   super();
   this.attachShadow({ type: 'open', slotChangeOptions: { ... });
}

This would bind the same options dict to observers on every slot in the newly created shadow tree, yay!

With the recent inclusion of Declarative Shadow DOM, a tree global option like this implies that we might need to expand the attributes available on the <template> element in that context. We could use a single attribute with a custom parser (like we see in srcset et al), which would allow for a single attribute:

<host-element>
  <template shadowroot="open" slotchangeoptions="...">
    <slot></slot>
  </template>
</host-element>

Or we could set more specific attributes:

<host-element>
  <template shadowroot="open" slotchangesubtree slotchangechildlist etc.>
    <slot></slot>
  </template>
</host-element>

I this approach, attributeFilter accepts a non-boolean data type, so a custom parser may be required in either approach.

Individual slots should opt-in

In many cases, a developer only wants to upgrade a single <slot> element to these capabilities, in keeping with the original intention of the specification, and in that case it might be better to submit the options dict directly on the <slot> element itself. This approach has the side benefit of allowing the same API no matter how your shadow tree is created, which likely adds implementor simplicity, but once again we have the option to apply the dict as a single attribute, which a custom parser:

<template>
   <slot options="..."></slot>
</template>

Or as multiple attributes:

<template>
   <slot subtree childlist etc.></slot>
</template>

Again, attributeFilter would likely need a custom parse in this context, but there is much more established art around collecting a list of strings than there is in collecting an object of booleans and arrays from an attribute.

Alternatives?

I'm sure there are a number of other ways to approach this, and I'd be happy to append them right here as they come to mind. Finding a way to simplify developer interactions with slotted content, and possibly open a way towards further simplicity and flexibility in this are is quite exciting.


Thanks in advance for your thoughtful feedback on this idea. I look forward to where the conversation takes us!

claviska commented 3 years ago

I fully agree that the first impression of how slotchange works tends to be "anything that changes in the slot will emit this event" and that's obviously not true. I also agree that having something to solve this more elegantly would be useful, so thank you for starting the discussion!

The issue seems to stem from the ambiguity of "change." What exactly is changing? With slotchange, it's not changes to the slotted element's attributes nor its children — in those cases, the element remains the same so, correctly, no event is emitted. Only when the slotted element itself is added/removed will the event be emitted. IMO, this behavior is correct and acceptable, albeit sometimes inconvenient.

What this proposes is similar to a partial reimplementation of the MutationEvent API, which was deprecated for performance reasons. Even if performance were a non-issue, the concept of customizing when or how events are emitted through options and/or attributes feels a bit weird. I can't think of any other APIs that work like this.

If this were to be supported, I would prefer not to allow this:

this.attachShadow({ type: 'open', slotChangeOptions: { ... });

And instead explicitly opt each slot in to avoid potential confusion.

<slot subtree childlist></slot>

While I don't think an event-based approach to this problem will be entertained due to performance reasons, it doesn't seem difficult to polyfill this behavior.

  1. Listen to slotchange on all slots
  2. Attach a mutation observer to each slotted element, emitting a custom event (e.g. slotmutated)
  3. Add a mutation observer to watch all slot attributes (e.g. subtree childlist etc.)
  4. Update the observer when a slot or any of its "option" attributes change

It would be a pretty handy utility that gives you the behavior you want without the performance implications.

Westbrook commented 3 years ago

IMO, this behavior is correct and acceptable, albeit sometimes inconvenient.

I agree that this is both "correct" and acceptable, beyond the gotcha that updating a node's textContent doesn't add/remove that node and as such does not trigger an slotchange event.


I also feel that applying directly on the <slot> element is likely the best path forward (where this taken up), but I wanted to include as many options as I could think of to make sure we didn't get stuck in a single yes/no conversation. I'd also go so far to say that if it weren't declarative (don't shoot, I'm unarmed) and you could only apply this imperatively, that could understand not wanting to pollute the attribute space. I'm not sure that's possible now, with the Declarative Shadow DOM spec as far along as it is, but I'm not closed to the option.


As for applying this functionality in user code (a version of polyfilling that doesn't imply that browsers will adopt or that the feature is somehow "not done" until they do), it is relatively straight forward. Its tight proximity to the capabilities of slotchange are what lead me to this conversation, over and over again, however.


the concept of customizing when or how events are emitted through options and/or attributes feels a bit weird. I can't think of any other APIs that work like this.

To help broaden my scope of knowledge, are there any other elements that both observe their "children" and report changes to those children in the platform today? I understand the slotchange event to represent a fairly unique (if not wholly unique) capability, and in that way feel like there's less of a "let's align with the rest of the platform" responsibility here. It could be that uniqueness (and it's association to the MutationEvent API) that prevents innovation in this area. Were that the case, maybe we should look at alternative APIs that were more likely to grow with the platform?

In that way, I'd say that while it doesn't report the changes to the outside that the <select> element already observes its children for changes beyond childList. It's likely not leveraging Mutation Observer, but if it were, it would absolutely require subtree, characterData, and maybe more to reactively update the UI it delivers when selected options have their light DOM content change. Maybe there's something to be learned there around internals as yet not surfaced to developers?

dbatiste commented 3 years ago

I ran into this exact issue a week or to ago, which required wiring up MutationObservers to the assigned (and flattened!) elements of a slot. 😬

It's entirely possible there is more than one handler wired up for slotchange associated with a particular slot. It seems a little odd that all such handlers would be subject to being invoked based on how the slot is defined. An alternative might be to somehow allow such options to be specified as part of the event wire-up.

castastrophe commented 3 years ago

Making some use-case notes for myself and thought I'd share. Below are events a developer might want to optionally observe:

  1. NodeItem is added to/removed from a slot a) textContent is added to/removed from a slot directly
  2. textContent is added to/removed from a NodeItem assigned to a slot
  3. Attribute is added/removed/changed on a node assigned to a slot
  4. NodeItem (child) is added to/removed from a NodeItem assigned to a slot
trusktr commented 3 years ago

While I don't think an event-based approach to this problem will be entertained due to performance reasons, it doesn't seem difficult to polyfill this behavior.

I don't understand why we think performance is an issue with events. If there are events attached, DOM change events can be handled synchronously, and that machinery can be enabled only if an event listener is added. Secondly deferring or batching event handling off of the synchronous events is literally no more difficult than using something like MutationObserver. MutationObserver has a worse developer experience than someone simply deferring their updates manually.

Furthermore, it would be simple to add an event option like {deferred: true}, and the developer experience would be yet even better.

I think we can make the web both easier to work with, and have performant options.

rniwa commented 3 years ago

While I don't think an event-based approach to this problem will be entertained due to performance reasons, it doesn't seem difficult to polyfill this behavior.

I don't understand why we think performance is an issue with events. If there are events attached, DOM change events can be handled synchronously, and that machinery can be enabled only if an event listener is added. Secondly deferring or batching event handling off of the synchronous events is literally no more difficult than using something like MutationObserver. MutationObserver has a worse developer experience than someone simply deferring their updates manually.

The biggest issue with sync DOM mutation events in the past has been security bugs. As a result, we're not willing to entertain the idea of adding a yet another sync DOM event like that. custom elements reaction callback is as synchronous as we'd ever tolerate for something like this.

trusktr commented 2 years ago

Maybe that's true of the MutationEvent implementation (got a link?), but in general the act of using an event emitter pattern does not automatically constitute a security concern.

Custom Element callbacks don't work on non-custom elements.

I believe an event pattern could be performant, secure, and have a better dev experience than the current status quo (MutationObserver).

rniwa commented 2 years ago

Maybe that's true of the MutationEvent implementation (got a link?), but in general the act of using an event emitter pattern does not automatically constitute a security concern.

I'm talking about security bugs in the browser engine, not security bugs in websites.

trusktr commented 2 years ago

Why is slotchange an event, and not a MutationObserver-like thing (f.e. SlotObserver perhaps) and batched/deferred? Slot changes are inherently the same as DOM MutationEvents, but for the composed tree.