solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.17k stars 918 forks source link

Custom elements callbacks should untrack #2039

Open titoBouzout opened 8 months ago

titoBouzout commented 8 months ago

Describe the bug

Custom elements have numerous callbacks and these should untrack to avoid recursion. This is a bit tricky because a lot of DOM methods for manipulation could cause the callbacks to fire.

Your Example Website or App

https://playground.solidjs.com/anonymous/b6031b3f-7ce9-4322-a06f-31823340af8d

Steps to Reproduce the Bug or Issue

Testing code is the following

import { render } from "solid-js/web";
import { createSignal } from "solid-js";

const [read, write] = createSignal(true);

function recurse(name, ...args) {
  console.log(name, ...args);
  write(!read());
}

class CustomElement extends HTMLElement {
  static observedAttributes = ["string-attribute"];

  constructor() {
    super();
    recurse("constructor");
  }
  connectedCallback() {
    recurse("Custom element added to page.");
  }

  disconnectedCallback() {
    recurse("Custom element removed from page.");
  }

  adoptedCallback() {
    recurse("Custom element moved to new page.");
  }

  attributeChangedCallback(name, oldValue, newValue) {
    recurse(`Attribute ${name} has changed.`, oldValue, newValue);
  }
  set boolean(value) {
    recurse(`boolean has changed.`, value);
  }
}

customElements.define("custom-element", CustomElement);

render(
  () => () => (
    <custom-element string-attribute="lala" boolean={true}>
      Test
    </custom-element>
  ),
  document.getElementById("app")!,
);
lxsmnsyc commented 8 months ago

I'd have to argue this is a no-fix. There's no way to intercept this at all. If anything, the responsibility must be delegated to the user.

titoBouzout commented 8 months ago

Its untracking:

  1. innerHTML (unless it's done in a template)
  2. any property/attribute set
  3. append/remove/after/before/etc
  4. createElement

What makes you think there's no way?

ryansolid commented 8 months ago

It"d definitely considerably increase size of codegen and put overhead on everything. There are a ton of touch points too because of all the micro optimizations we do. This seems pretty tricky. I doubt highly the vast majority of Solid users would want to take any sort of hit for WCs. To be fair it isn even WCs in general as Solid Element works fine doing the untrack itself. Its this specific pattern.

titoBouzout commented 8 months ago

Interesting, I only used this because I know there's an effect there. So if one were to create custom elements using solid it will be using Solid Element. I wonder if in that case it will still track, as there are so many ways to cause these callbacks. As every day there's more use of WE this should be keep in consideration in my opinion.