Reactive root per element, and workaround for Solid.js issue preventing some effects from running #67

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.

Sign up for GitHub

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

Merged

trusktr merged 3 commits into main from reactive-root-per-element

Jan 25, 2026

dist/LumeElement.d.ts.map

Large diffs are not rendered by default.

dist/LumeElement.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

dist/LumeElement.js.map

Large diffs are not rendered by default.

dist/LumeElement.test.js.map

Large diffs are not rendered by default.

dist/decorators/attribute.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

dist/decorators/attribute.test.js.map

Large diffs are not rendered by default.

dist/decorators/element.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

dist/decorators/element.js.map

Large diffs are not rendered by default.

dist/decorators/element.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

dist/decorators/element.test.js.map

Large diffs are not rendered by default.

src/LumeElement.test.ts

-Original file line number
+Diff line change
@@ -1,4 +1,4 @@
-    import {createEffect} from 'solid-js'
+    import {createEffect, createSignal} from 'solid-js'
     import html from 'solid-js/html'
     import {effect, memo, signal, signalify} from 'classy-solid'
     import {Element, element, type AttributeHandlerMap} from './index.js'
@@ Expand Down Expand Up / @@ -1024,4 +1024,50 @@ describe('LumeElement', () => { @@
     		expect(el.effectCount).toBe(4, 'effect should run on reactive changes after reconnecting')
     		expect(el.effectCount2).toBe(5, 'old-style effect should run on reactive changes after reconnecting')
     	})
+    	it('does not keep elements in template from managing their own effect life cycle', () => {
+    		const [count, setCount] = createSignal(0)
+    		@element('parent-el')
+    		class ParentEl extends Element {
+    			override template = () => html`<child-el></child-el>`
+    		}
+    		@element('child-el')
+    		class ChildEl extends Element {
+    			effectRuns = 0
+    			value = 0
+    			@effect testEffect() {
+    				this.value = count() // track
+    				this.effectRuns++
+    			}
+    		}
+    		const parent = new ParentEl()
+    		document.body.append(parent)
+    		els.push(parent)
+    		const child = parent.shadowRoot!.querySelector('child-el') as ChildEl
+    		expect(child.effectRuns).toBe(1)
+    		expect(child.value).toBe(0)
+    		setCount(1)
+    		expect(child.effectRuns).toBe(2)
+    		expect(child.value).toBe(1)
+    		child.remove()
+    		setCount(2)
+    		expect(child.effectRuns).toBe(2, 'effect should not run when disconnected')
+    		expect(child.value).toBe(1, 'value should not change when disconnected')
+    		document.body.append(child)
+    		expect(child.effectRuns).toBe(3, 'effect should run once on reconnect')
+    		expect(child.value).toBe(2, 'value should update on reconnect')
+    		setCount(3)
+    		expect(child.effectRuns).toBe(4, 'effect should run on reactive changes after reconnecting')
+    		expect(child.value).toBe(3, 'value should update on reactive changes after reconnecting')
+    	})
     })

src/LumeElement.ts

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -1,4 +1,5 @@
  
    import {render} from 'solid-js/web'

    import {createRoot, getOwner, runWithOwner, type Owner} from 'solid-js'

    // isPropSetAtLeastOnce__ was exposed by classy-solid specifically for

    // @lume/element to use. It tells us if a signal property has been set at

    // least once, and if so allows us to skip overwriting it with a custom

    @@ -333,10 +334,29 @@ class LumeElement extends HTMLElement {
  
    	#effects = new Effects()

    	#disposeRoot?: () => void

    	#reactiveRoot_?: Owner

    	get #reactiveRoot() {

    		if (this.#reactiveRoot_) return this.#reactiveRoot_

    		createRoot(dispose => {

    			this.#reactiveRoot_ = getOwner()!

    			this.#disposeRoot = () => {

    				dispose()

    				this.#reactiveRoot_ = undefined

    				this.#disposeRoot = undefined

    			}

    		})

    		return this.#reactiveRoot_!

    	}

    	// For old-style (non-decorator) effects (f.e. subclasses creating effects

    	// in connectedCallback).

    	createEffect(fn: () => void) {

    		this.#effects.createEffect(fn)

    		runWithOwner(this.#reactiveRoot, () => this.#effects.createEffect(fn))

    	}

    	#disposeTemplate?: () => void

    @@ -350,12 +370,13 @@ class LumeElement extends HTMLElement {
  
    			)

    		this.#setStyle()

    		startEffects(this) // start new-style (decorator) effects

    		runWithOwner(this.#reactiveRoot, () => startEffects(this)) // start new-style (decorator) effects

    	}

    	disconnectedCallback() {

    		this.#effects.clearEffects() // Clean up old-style (non-decorator) effects.

    		stopEffects(this) // Clean up new-style (decorator) effects

    		this.#disposeRoot?.()

    		this.#disposeTemplate?.()

    		this.#cleanupStyle()

    	}

src/decorators/attribute.test.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -796,7 +796,6 @@ describe('@attribute', () => { @@
     				@attribute foo = '0'
     				@effect testEffect() {
-    					console.log('effect running')
     					count++
     					value = this.foo
     				}
@@ Expand Down @@

src/decorators/element.test.ts

-Original file line number
+Diff line change
@@ -1,4 +1,6 @@
-    import {createEffect} from 'solid-js'
+    import {createEffect, createSignal} from 'solid-js'
+    import html from 'solid-js/html'
+    import {render} from 'solid-js/web'
     import {signal} from 'classy-solid'
     import {
     	Element,
@@ Expand Down Expand Up / @@ -642,6 +644,55 @@ describe('@element decorator', () => { @@
     			}).toThrow('@element is only for use on classes.')
     		})
     	})
+    	// https://github.com/solidjs/solid/issues/2580
+    	it('works around a Solid.js bug causing some effects to never run', () => {
+    		@element('drippy-scene')
+    		class DrippyScene extends Element {
+    			effectRuns = 0
+    			override connectedCallback() {
+    				super.connectedCallback()
+    				// This never runs without the untrack workaround in the @element decorator:
+    				createEffect(() => {
+    					this.effectRuns++
+    				})
+    			}
+    		}
+    		DrippyScene
+    		const [someValue, setSomeValue] = createSignal(false)
+    		@element('bottom-sheet')
+    		class BottomSheet extends HTMLElement {
+    			connectedCallback() {
+    				if (!someValue()) setSomeValue(true)
+    			}
+    		}
+    		BottomSheet
+    		render(
+    			() => html`
+    				<drippy-scene></drippy-scene>
+    				${
+    					// this empty text content interpolation site, along with the read/write of someValue above, causes the drippy-scene effect to never run without the untrack workaround in the @element decorator
+    					() => ''
+    				}
+    				<bottom-sheet></bottom-sheet>
+    			`,
+    			document.body,
+    		)
+    		const el = document.body.querySelector('drippy-scene') as DrippyScene
+    		expect(el.effectRuns).toBe(1)
+    		document.body.querySelector('bottom-sheet')!.remove()
+    		el.remove()
+    	})
     })
     function testAttributes(
@@ Expand Down @@

src/decorators/element.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -459,6 +459,15 @@ function applyElementDecoration( @@
     				handlePreUpgradeValues(this)
     			})
     		}
+    		override connectedCallback() {
+    			// Workaround for Solid.js bug causing some effects to never run
+    			// when a component is created during another component's render.
+    			// See the test "works around a Solid.js bug causing some effects to
+    			// never run" in element.test.ts.
+    			// https://github.com/solidjs/solid/issues/2580
+    			untrack(() => super.connectedCallback?.())
+    		}
     	}
     	if (metadata) {
@@ Expand Down @@