webcomponents / custom-elements-everywhere

Custom Element + Framework Interoperability Tests.
https://custom-elements-everywhere.com
Other
1.19k stars 103 forks source link

New test: custom elements should not cause infinite reactivity loops in signals-based libraries #2324

Open trusktr opened 10 months ago

trusktr commented 10 months ago

This test is namely for signals-based libraries. We need a test that shows that when a custom elements is rendered, none of its life cycle methods (constructor, connectedCallback, adoptedCallback, disconnectedCallback, and attributeChangedCallback) cause an infinite loop when reading the framework's reactive state.

Here's an example (and a good sample test case we can write) in Solid.js where it will infinite-loop and eventually crash by simply reading a signal in a custom element life cycle method:

Besides this, we may want to include more libs, or lib combos, that use signals-and-effects patterns:

titoBouzout commented 10 months ago

I updated the test

import {
  render,
  signal,
  effect,
  untrack,
  setAttribute,
  setProperty
} from 'pota'

const [read, write] = signal(true)

function recurse(name) {
  console.log(name)
  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 has changed.')
  }
  set boolean(value) {
    recurse('boolean has changed.')
  }
}

customElements.define('custom-element', CustomElement)

/** Make sure the test is done inside an effect */

let dispose

effect(() => {
  // if the reactive lib tracks any custom element callbacks, it will recurse.

  /**
   * "document.createElement" is not controlled by the reactive libs, so
   * just untrack it.
   */
  const element = untrack(() =>
    document.createElement('custom-element')
  )

  /** Reactive lib shouldnt cause tracking when setting an attribute */
  setAttribute(element, 'string-attribute', 'lalala 2')

  /** Reactive lib shouldnt cause tracking when setting a property */
  setProperty(element, 'boolean', true)

  /** Reactive lib shouldnt cause tracking when rendering */
  dispose = render(element)

  /** Reactive lib shouldnt cause tracking when removed */
  dispose()
})
trusktr commented 9 months ago

@titoBouzout Looks good. I'm not sure about set boolean, as making an infinitely-looping property is probably just not wanted no matter what the scenario is. Not sure we can guard against that particular case. But definitely the life cycle methods.