Merge pull request #10 from lume/fix-duplicate-signals

fix: ensure that signalifying properties already signalified by the @signal decorator does not create double signals

dist/signals/signalify.d.ts

@@ -53,6 +53,7 @@ export declare function signalify<T extends object>(obj: T, ...props: (keyof T)[
 export declare function signalify<T extends object>(obj: T, ...props: [key: keyof T, initialValue: unknown][]): T;
 export declare function __isPropSetAtLeastOnce(instance: object, prop: string | symbol): boolean;
 export declare function __trackPropSetAtLeastOnce(instance: object, prop: string | symbol): void;
+export declare const isSignalGetter: WeakSet<Function>;
 export declare function __createSignalAccessor<T extends object>(obj: T, prop: Exclude<keyof T, number>, initialVal: unknown): void;
 export declare function __getSignal(obj: object, storage: WeakMap<object, SignalFunction<unknown>>, initialVal: unknown): SignalFunction<unknown>;
 //# sourceMappingURL=signalify.d.ts.map

src/decorators/signal.test.ts

@@ -2,6 +2,7 @@ import {createEffect} from 'solid-js'
 import {testButterflyProps} from '../index.test.js'
 import {reactive} from './reactive.js'
 import {signal} from './signal.js'
+import {signalify} from '../signals/signalify.js'
 
 describe('classy-solid', () => {
 	describe('@reactive, @signal', () => {
@@ -359,5 +360,47 @@ describe('classy-solid', () => {
 			expect(count).toBe(1)
 			expect(b!).toBe(b2)
 		})
+
+		it.only('prevents duplicate signals for any property', () => {
+			@reactive
+			class Insect {
+				@signal venomous = 0
+
+				@signal accessor legs = 6
+
+				#eyes = 10
+				@signal get eyes() {
+					return this.#eyes
+				}
+				@signal set eyes(n) {
+					this.#eyes = n
+				}
+
+				antennas = 0
+
+				constructor() {
+					// This should not add any extra signals for properties that
+					// are already signalified by the @signal decorator
+					signalify(this, 'venomous', 'legs', 'eyes', 'antennas')
+				}
+			}
+			const i = new Insect()
+
+			testNoDuplicateSignal(i, 'venomous')
+			testNoDuplicateSignal(i, 'legs')
+			testNoDuplicateSignal(i, 'eyes')
+			testNoDuplicateSignal(i, 'antennas')
+
+			function testNoDuplicateSignal(o: Insect, prop: keyof Insect) {
+				let count = 0
+				createEffect(() => {
+					count++
+					o[prop]
+				})
+				expect(count).toBe(1)
+				o[prop]++
+				expect(count).toBe(2) // it would be 3 if there were an extra signal
+			}
+		})
 	})
 })

src/decorators/signal.ts

@@ -1,5 +1,5 @@
 import {$PROXY} from 'solid-js'
-import {__getSignal, __trackPropSetAtLeastOnce, __createSignalAccessor} from '../signals/signalify.js'
+import {__getSignal, __trackPropSetAtLeastOnce, __createSignalAccessor, isSignalGetter} from '../signals/signalify.js'
 import type {PropKey, PropSpec} from './types.js'
 import type {SignalFunction} from '../signals/createSignalFunction.js'
 
@@ -77,12 +77,9 @@ export function signal(
 					let initialValue = propSpec.initialValue
 
 					// @prod-prune
-					if (!Object.hasOwn(this, prop))
-						// continue
-						throw new PropNotFoundError(prop)
+					if (!Object.hasOwn(this, prop)) throw new PropNotFoundError(prop)
 
 					__createSignalAccessor(this as any, prop, initialValue)
-					// CONTINUE testing this way of finalizing signal fields
 				}
 				return
 			}
@@ -95,7 +92,7 @@ export function signal(
 		const signalStorage = new WeakMap<object, SignalFunction<unknown>>()
 		let initialValue: unknown = undefined
 
-		return {
+		const newValue = {
 			init: function (this: object, initialVal: unknown) {
 				initialValue = initialVal
 				return initialVal
@@ -112,6 +109,10 @@ export function signal(
 				s(typeof newValue === 'function' ? () => newValue : newValue)
 			},
 		}
+
+		isSignalGetter.add(newValue.get)
+
+		return newValue
 	} else if (kind === 'getter' || kind === 'setter') {
 		const getOrSet = value as Function
 		const initialValue = Undefined
@@ -134,10 +135,14 @@ export function signal(
 			pairs[name] ??= 0
 			pairs[name]++
 
-			return function (this: object): unknown {
+			const newGetter = function (this: object): unknown {
 				__getSignal(this, signalStorage, initialValue)()
 				return getOrSet.call(this)
 			}
+
+			isSignalGetter.add(newGetter)
+
+			return newGetter
 		} else {
 			pairs[name] ??= 0
 			pairs[name]++

src/signals/signalify.test.ts

@@ -57,7 +57,7 @@ describe('classy-solid', () => {
 
 						// A deeper effect will be reading the property.
 						createEffect(() => {
-							console.log(obj.n)
+							obj.n
 						})
 					})
 

src/signals/signalify.ts

@@ -96,7 +96,7 @@ export function __trackPropSetAtLeastOnce(instance: object, prop: string | symbo
 	propsSetAtLeastOnce.get(instance)!.add(prop)
 }
 
-const isSignalGetter = new WeakSet<Function>()
+export const isSignalGetter = new WeakSet<Function>()
 
 export function __createSignalAccessor<T extends object>(
 	obj: T,