fix: ref on dynamic elements with reactive props (#772)

Author: anubra266

.changeset/quick-ref-dynamic-elements.md

@@ -0,0 +1,6 @@
+---
+'ripple': patch
+---
+
+Fix ref handling for dynamic elements with reactive spread props to avoid
+read-only/proxy symbol errors and prevent unnecessary ref teardown/recreation.

packages/ripple/src/runtime/internal/client/render.js

@@ -1,7 +1,7 @@
 /** @import { Block } from '#client' */
 
 import { destroy_block, ref } from './blocks.js';
-import { REF_PROP } from './constants.js';
+import { DESTROYED, REF_PROP } from './constants.js';
 import {
 	get_descriptors,
 	get_own_property_symbols,
@@ -252,6 +252,9 @@ export function apply_element_spread(element, fn) {
 	/** @type {Record<string | symbol, (() => void) | undefined>} */
 	var remove_listeners = {};
 
+	/** @type {Record<symbol, any>} */
+	var prev_symbols = {};
+
 	return () => {
 		var next = fn();
 
@@ -262,19 +265,28 @@ export function apply_element_spread(element, fn) {
 			}
 		}
 
+		/** @type {Record<symbol, any>} */
+		var current_symbols = {};
+
 		for (const symbol of get_own_property_symbols(next)) {
 			var ref_fn = next[symbol];
-
-			if (symbol.description === REF_PROP && (!(symbol in prev) || ref_fn !== prev[symbol])) {
-				if (effects[symbol]) {
+			current_symbols[symbol] = ref_fn;
+
+			if (
+				symbol.description === REF_PROP &&
+				(!(symbol in prev_symbols) ||
+					ref_fn !== prev_symbols[symbol] ||
+					(effects[symbol] && (effects[symbol].f & DESTROYED) !== 0))
+			) {
+				if (effects[symbol] && (effects[symbol].f & DESTROYED) === 0) {
 					destroy_block(effects[symbol]);
 				}
 				effects[symbol] = ref(element, () => ref_fn);
 			}
-
-			next[symbol] = ref_fn;
 		}
 
+		prev_symbols = current_symbols;
+
 		for (let key in remove_listeners) {
 			// Remove event listeners that are no longer present
 			if (!(key in next) && remove_listeners[key]) {

packages/ripple/tests/client/dynamic-elements.test.ripple

@@ -593,6 +593,141 @@ describe('dynamic DOM elements', () => {
 		expect(innerScopes).toHaveLength(0);
 	});
 
+	it('handles ref on dynamic element passed through component with reactive props', () => {
+		let capturedElement: HTMLElement | null = null;
+		let refCallCount = 0;
+
+		component Button(props: any) {
+			const el = track('button');
+			<@el {...props} />
+		}
+
+		component App() {
+			let active = track(false);
+
+			<Button
+				data-active={String(@active)}
+				onClick={() => {
+					@active = !@active;
+				}}
+				{ref (el: HTMLElement) => {
+					capturedElement = el;
+					refCallCount++;
+				}}
+			>
+				{'content'}
+			</Button>
+		}
+
+		render(App);
+		flushSync();
+
+		expect(capturedElement).toBeTruthy();
+		expect(capturedElement!.tagName).toBe('BUTTON');
+		expect(capturedElement!.getAttribute('data-active')).toBe('false');
+		const initialRefCount = refCallCount;
+
+		// Click the button to trigger reactive prop update
+		capturedElement!.click();
+		flushSync();
+
+		// After clicking, the reactive prop should update without error
+		expect(capturedElement!.getAttribute('data-active')).toBe('true');
+		// Ref block should not have been recreated on prop update
+		expect(refCallCount).toBe(initialRefCount);
+	});
+
+	it('handles ref on dynamic element with spread props containing reactive values', () => {
+		let capturedElement: HTMLElement | null = null;
+
+		component Button(props: any) {
+			const el = track('button');
+			<@el {...props} />
+		}
+
+		component App() {
+			let active = track(false);
+
+			let buttonProps = track(
+				() => ({
+					'data-active': @active,
+				}),
+			);
+
+			<Button
+				{...@buttonProps}
+				onClick={() => {
+					@active = !@active;
+				}}
+				{ref (el: HTMLElement) => {
+					capturedElement = el;
+				}}
+			>
+				{'content: '}
+				{@active}
+			</Button>
+		}
+
+		render(App);
+		flushSync();
+
+		expect(capturedElement).toBeTruthy();
+		expect(capturedElement!.tagName).toBe('BUTTON');
+
+		// Click the button to trigger reactive update
+		capturedElement!.click();
+		flushSync();
+
+		// Should not throw, and ref should still be valid
+		expect(capturedElement!.tagName).toBe('BUTTON');
+	});
+
+	it('re-establishes ref with cleanup after parent block re-runs', () => {
+		let cleanupCount = 0;
+		let refCallCount = 0;
+		let capturedElement: HTMLElement | null = null;
+
+		component Button(props: any) {
+			const el = track('button');
+			<@el {...props} />
+		}
+
+		component App() {
+			let active = track(false);
+
+			<Button
+				data-active={String(@active)}
+				onClick={() => {
+					@active = !@active;
+				}}
+				{ref (el: HTMLElement) => {
+					capturedElement = el;
+					refCallCount++;
+					return () => {
+						cleanupCount++;
+					};
+				}}
+			>
+				{'content'}
+			</Button>
+		}
+
+		render(App);
+		flushSync();
+
+		expect(capturedElement).toBeTruthy();
+		expect(refCallCount).toBe(1);
+		expect(cleanupCount).toBe(0);
+
+		// Click to trigger reactive prop update
+		capturedElement!.click();
+		flushSync();
+
+		// Ref with cleanup should be re-established after parent teardown cycle
+		expect(capturedElement!.getAttribute('data-active')).toBe('true');
+		expect(refCallCount).toBeGreaterThanOrEqual(1);
+	});
+
 	it('should remove and add back a text node in a conditional statement with a tracked', () => {
 		component App() {
 			let b = track(true);