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

Custom elements callbacks should untrack #2039

Open
titoBouzout opened this issue Jan 19, 2024 · 4 comments
Open

Custom elements callbacks should untrack #2039

titoBouzout opened this issue Jan 19, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@titoBouzout
Copy link
Contributor

titoBouzout commented Jan 19, 2024

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
Copy link
Member

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
Copy link
Contributor Author

titoBouzout commented Jan 19, 2024

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
Copy link
Member

ryansolid commented Jan 19, 2024

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
Copy link
Contributor Author

titoBouzout commented Jan 19, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants