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

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

Open
trusktr opened this issue Jan 20, 2024 · 2 comments

Comments

@trusktr
Copy link
Contributor

trusktr commented Jan 20, 2024

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:

  • Solid.js
  • Preact with Preact Signals
  • Lit with its wrapper around Preact Signals, @lit-labs/preact-signals
  • React + MobX and others
  • Vue with ref() API and setup mode
  • Svelte 5 with its new Runes API
  • etc
@trusktr trusktr changed the title New test: custom elements should not cause infinite reactivity loops New test: custom elements should not cause infinite reactivity loops in signals-based libraries Jan 20, 2024
@titoBouzout
Copy link

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

trusktr commented Jan 30, 2024

@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.

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

No branches or pull requests

2 participants