Re: [WICG/webcomponents] The is="" attribute is confusing? Maybe we should encourage only ES6 class-based extension. (#509)

> the fact the API mimics CE but needs a suffix parameter as element is a bit yack

Now sure what you mean by that. Are you talking about `constructor` receiving the `element` arg? If so, more on that below regarding the `this.target` idea. Passing it in via `constructor` does mean users have to assign it to their own (possibly `#private`) property to use it in other methods.

There's only one issue with the `this.target` idea: how does the engine provide the value if the classes don't extend from a base class? The engine would have either

- construct the behavior instance, _then_ assign the value, which means it can't be used in the constructor (that's originally how I had it, but then I wanted to use the elements in the behavior constructors).
- or modify the class prototype, which means that after passing the class to `elementBehaviors.define`, the class shape would be different than what the class definition shows (so not good for static analysis).
- or provide a base `Behavior` class, then users would be required to extend from it, and the base class would have `target` as part of its shape.
  - EDIT: Oh, is this what you meant about `class CustomBehavior {`? That this base class in built in and all users are required to extend from it? If so, then yeah that would work to prevent the constructor arg. 

> as soon as one behavior extends a base behavior, it's easy to think that class definition is for a CE, not a behavior

True. I see the ambiguity there. A developer should be responsible about knowing what their class does though...

> as this is based on MutationObserver, I guess everything is inevitably asynchronous, which leads to unexpected sequence of events (the CE polyfill with MO has the same gotcha more than a developer felt with)


True. If it were to become a spec'd feature, that could be changed to be sync during parse like with CEs.

> My last points regard the fact that connectedCallback might be called when disconnectedCallback also happened

That can't happen. If an element is disconnected already, connectedCallback will not be called. The sequence can only ever be `connectedCallback` -> `disconnectedCallback` -> `connectedCallback` -> `disconnectedCallback` -> etc. If that's not the case, it would be a bug.



> this is exactly what wickedElements has been for years, except the definition is via CSS selector, not specific attribute

That's also an interesting approach. In some ways it is like jQuery (instantiating plugins via selectors).



> `customBehaviors` looks like a better name, if it wants to be similar to `customElements` in terms of API

I thought about that, but there's no already-existing built-in behaviors, so they aren't really "custom".



> the `disconnectedCallback` called when the `has` attribute changes and the behavior is removed is misleading, as it's not about the element being disconnected but about the behavior being disconnected. Unless the intention is indeed to simply have connected and disconnected run **only** when the behavior is added or removed, I find this part hard to reason about.

I thought about this too. Basically it's more like a "when do I clean up?". Rather than having, for example, two separate methods, I just think of it this way: if an element is removed from the live DOM, or if the behavior is removed from an element, then effectively the behavior is disconnected from live DOM.

The behaviors (currently) can not be moved from one element to another. So even after being disconnected from DOM, `this.element` could still reference the owner element.

Also, not sure we'd want the behavior to still be "connected", because if it isn't in the DOM, it really shouldn't do anything (it should be cleaned up, just like the element was.



> similarly, the `connectedCallback`. It's not clear if it should be called when the element attribute changes (i.e. via element.behaviors.add('behavior')) even if the element is offline.

Same thought here: is the behavior connected into the DOM? If so, then it is guaranteed to have been connected to an element, and it should initialize things at that time. The main thoughts are "when to initialize" and "when to cleanup".

Could there be a different name for those two methods?



> if these callbacks are bound to the element lifecycle, the disconnectedCallback for cleanup sounds footgun proof, because when a behavior is removed (i.e. via behaviors.remove('behavior')), but the element is still live, it looks like we're simulating a destructor instead, not a disconnect.


The "destructor" is basically what I was going for. The "cleanup" is not necessarily tied to the element's life cycle _exactly_. F.e. remove behavior -> behavior cleanup, or remove element -> behavior cleanup.

Suppose instead `elementDisconnectedCallback` is only for when the element is disconnected, and then we have a new method like `behaviorDisconnectedCallback` for when a behavior is removed from an element (and similar with `elementConnectedCallback` and `behaviorConnectedCallback`). During parse, both connected methods would fire (so somewhat redundant). What if an element is removed, but the behavior is not, and it didn't clean up on element disconnect?

Would this be better? Maybe people would be likely to do things wrong compared to just "initialize" and "cleanup".

My inclination is that I will most likely just have both of the disconnect call a third custom `cleanup` method.

What about `initializeCallback` and `cleanupCallback`? Even with those names, the documentation would still need to describe how it works (initialization happens on element or behavior connect, and clean up happens on element or behavior disconnect).



> it's not clear if offline elements will work as expected (at least with a MO that observes only live nodes)

Currently if the element is disconnected, the behavior was cleaned up (disconnectedCallback) and so nothing should run (ideally). Not sure if `attributeChangedCallback` should continue to run. I haven't relied on it running after a behavior is cleaned up (disconnected), so I don't even know if it does or not currently.



> it's not clear if trashing layout would invoke disconnected (but it'd expect so)

What do you mean? Does trashing layout mean getting rid of a subtree? If so, then all behaviors would have `disconnectedCallback` ran.



> it's not clear if injecting in templates via html would connect behaviors, or only once document.importNode(template.content, true) happens

Do you mean with `innerHTML`, for example? If so, then it should connected them. Any time an element exists with a `has=""` attribute present for whatever reason, the behaviors needs to be instantiated if they have been defined. If that doesn't happen, there's a bug.

Or do you mean behaviors on elements that are inside a `<template>`? I've never tried that, so not sure what will happen currently. But it should be the same as CEs: behaviors should not be instantiated (just like elements are not upgraded) until they are in a real DOM.

EDIT: I just did a test, and it appears if elements are in a `<template>` nothing happens (good).

> it's not clear if adding and removing the same behavior would use the same instance or create a new one


It just creates a new one currently. Actually exposing `behaviors` on the element is not good at the moment, because modifying that Set will not do anything (I simply placed it there so I could easily get an instance if I wanted to (f.e. in devtools debugging).

But perhaps that should be read-only, and perhaps the only way to add/remove them imperatively would be something like `element.behaviors.add('behavior-name')` and `element.behaviors.remove('behavior-name')` similar to `element.classList`.

Otherwise, if a user instantiates them manually, we couldn't guarantee that they pass in the element instance to the constructor (relating to the `this.target` ideas above).

I'll circle back to modifying element-behaviors to fix that part regarding `behaviors`. I think it is can be useful to get the instances, but perhaps they should not be instantiable.

Another idea regarding `this.target` is, maybe `connectedCallback` would receive the `element`, so `connectedCallback(element) {}`. If it was like that, then the user could instantiate the behaviors, and add them by reference, `element.behaviors.add(theBehavior)`.

> This allows behaviors to initialize their listeners when attached, and cleanup once detached, but these operations are not necessarily bound with the element live state, simplifying setup/teardown per each behavior, less unexpected when `disconnectedCallback` is called, but the element is still live in the DOM (which is the most common case here, when behaviors are dropped).
> 
> All names are subject to changes, but I think this proposal would be much cleaner than your one, and simply because, differently from Custom Elements, behaviors can be added and removed at runtime, while custom elements are forever, and strictly bound with the element these represent.
> 
> I hope this sounds better to you too.

I'm not sure. There's something nice about the simple initialize vs cleanup. The user need not worry about if the element still is in the DOM or not. The behavior just does what it does when it needs to, otherwise it is cleaned up and gone. 

Also is "attached" vs "connected" naming clear? Both terms have been used to describe when elements are connected to the DOM.

I do agree the new suggestion would allow more flexibility of some sorts (partial clean up rather than full cleanups). Is it worth the extra methods? What may we want to do with the extra methods (that would be worth it) that we can't do with only two methods regardless of name?

With four methods, I would imagine some people just doing this:

```js
class MyBehavior {
  connectedCallback() {
    this.initialize()
  }
  attachedCallback() {
    this.initialize()
  }
  initialized = false
  initialize() {
    if (this.initialized) return
    this.initialized = true
    // initialize stuff.
  }
  disconnectedCallback() {
    this.cleanup()
  }
  detachedCallback() {
    this.cleanup()
  }
  cleanup() {
    if (!this.initialized) return
    this.initialized = false
    // cleanup stuff.
  }
}
```

I think most of the time I would just want to initialize things, it will run for a while, then finally cleanup.

I think that I like the two-methods better still. But I'm not clear on if I'd actually want to do something different with the four method that merely forwarding to initialize/cleanup. I think I like the idea of "the behavior is connected or disconnected from the DOM", regardless of whether it happened with the element or not.

In this form, the user does have a guarantee: `constructor` or `connectedCallback` always reveals a connected element available to the behavior author. Or basically, if the behavior is connected, the element must be too. If the behavior is disconnected, the element may or may not be connected, but this should be of no concern to the behavior which should clean itself up.

> `disconnectedCallback` might use a `detached = false`

That might be more confusing. I think four methods would be cleaner if it went that direction.

> this distinction between being removed, as behavior, or signaling the element was disconnected, looks kinda important to me

Wondering what are some compelling use cases for it.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/WICG/webcomponents/issues/509#issuecomment-805529408