WebReflection / regular-elements

Custom Elements made available for any node, and through CSS selectors
https://webreflection.github.io/regular-elements/test/
ISC License
90 stars 3 forks source link

Loses dynamically matched elements #21

Closed dy closed 4 years ago

dy commented 4 years ago

If we make element match selector dynamically, regular-elements/wicked-elements don't init that:

import wels from "wicked-elements";

wels.define(".x", {
  init() {
    console.log("init");
  },
  onconnected() {
    console.log("connected");
  }
});

let el = document.createElement("div");
document.body.appendChild(el);

setTimeout(() => {
  el.classList.add("x");
}, 100);

sandbox.

Direct mutation observer works fine

var observer = new MutationObserver((mutationList, observer) => {
  console.log(...mutationList)
});
observer.observe(el, {childList: true, attributes: true, subtree: true})
document.body.appendChild(el);
WebReflection commented 4 years ago

Try this instead:

import wels from "wicked-elements";
const $x = {
  init() {
    console.log("init");
  },
  onconnected() {
    console.log("connected");
  }
};
wels.define(".x", $x);

// further down ...
let el = document.createElement("div");
document.body.appendChild(el);

setTimeout(() => {
  el.classList.add("x");
  wels.define(el, $x);
}, 100);

The long story short, is that observing all attributes that could change on the DOM is a performance disaster, so since you are in charge of adding classes at runtime, you can as well use .define(element, definition) when that happens.

If you are not in charge, IIRC, the element will be upgraded whenever another element gets inserted, as regularElements work on inserted nodes.

Accordingly, this looks like a won't fix so far, so please let me know if my proposed solution works.

dy commented 4 years ago

Got it.

performance disaster

I wonder if that's the case for CSS selectors as well - intuitively they provide similar reaction (rerendering a bunch of elements) on any DOM mutations, aren't them?

The lib I'm building (spect) acts in a way similar to declarative set of rules, like CSS. I'd have to manually check all doc mutations then init regular elements, so that won't be performant solution either.

I think that's a question of benchmark. For now I guess I have to opt for selector-observer.

Thanks.

WebReflection commented 4 years ago

I wonder if that's the case for CSS selectors as well

It doesn't matter, MutationObserver would trigger gazillion times more than it does when no attributes are observed, but it's true that filtering these by class attribute could be a decent compromise.

I'll re-open this, and keep an eye on noticeable performance regressions, as I don't think using another library would solve performance issues, since such library would be based on exactly same primitives.

WebReflection commented 4 years ago

P.S. do you also expect all nodes that don't match a selector anymore to be somehow freed from the logic? 'cause that's the biggest limitation I currently see in both regular/wickedElements, you can't drop an element from it's definition, so that once initialized, it'll keep doing whatever, if it doesn't match that selector anymore (but this might need a huge refactoring all over the place, so it might take a while).

dy commented 4 years ago

all nodes that don't match a selector anymore to be somehow freed from the logic

That's a good point.

I decided for elements initialized directly $(realNodes, handler) to permanently have assigned behavior, whereas for selectors $('.behavior', handler) to lose the behavior when elements are disconnected, basically the ondisconnected callback turns off the assigned aspect (same way the CSS would act).

I wonder if that's possible though - spect uses augmentor as hooks provider by default, and there doesn't seem to be a way to dispose assigned hooks, is there? Ah, dropEffect, I see.

WebReflection commented 4 years ago

yes, dropEffect is used in neverland too, have a look in case it helps

WebReflection commented 4 years ago

Sorry, I meant dom-augmentor, not neverland, but basically if you use dom-augmentor instead of just augmentor, you'll have hooks pretty much 1:1 with React out of the box.

WebReflection commented 4 years ago

So, after thinking about it quite a while, but also prototyping a bit, I believe firing the whole circus per each possible attribute would make the library itself unusable, or better, a performance nightmare.

In details: I cannot just listen to classes change, for the simple reason that in CSS [any-attr=value] is a valid selector too.

As summary, whenever you want to create at runtime any reguler/wickedElement, you are in charge of also defining its behavior, otherwise you have to wait for any other node to be inserted on the page, so that the circus works out of the box.

As after years this runtime gotcha has came up only once, I'll close this as won't fix for the mentioned reasons, and since you have a workaround that wouldn't kill this library performance for everyone else that never had such issue, I hope that's sensible/reasonable 👋