From d1a566f97c8947addb994a3d338765edb2c8d68d Mon Sep 17 00:00:00 2001 From: Lin Tao <3520043790@qq.com> Date: Thu, 2 Apr 2026 17:09:04 +0800 Subject: [PATCH 1/2] fix(signal): prevent repeated observer registration in loops --- packages/solid/src/reactive/signal.ts | 77 +++++++++++++++------------ packages/solid/test/signals.spec.ts | 19 +++++++ 2 files changed, 61 insertions(+), 35 deletions(-) diff --git a/packages/solid/src/reactive/signal.ts b/packages/solid/src/reactive/signal.ts index a7b4995ef..713d2da28 100644 --- a/packages/solid/src/reactive/signal.ts +++ b/packages/solid/src/reactive/signal.ts @@ -86,6 +86,7 @@ export interface SignalState extends SourceMapValue { value: T; observers: Computation[] | null; observerSlots: number[] | null; + lastObserver: Computation | null; tValue?: T; comparator?: (prev: T, next: T) => boolean; // development-only @@ -157,17 +158,17 @@ export function createRoot(fn: RootFunction, detachedOwner?: typeof Owner) ? { owned: null, cleanups: null, context: null, owner: null } : UNOWNED : { - owned: null, - cleanups: null, - context: current ? current.context : null, - owner: current - }, + owned: null, + cleanups: null, + context: current ? current.context : null, + owner: current + }, updateFn = unowned ? IS_DEV ? () => - fn(() => { - throw new Error("Dispose method must be an explicit argument to createRoot function"); - }) + fn(() => { + throw new Error("Dispose method must be an explicit argument to createRoot function"); + }) : fn : () => fn(() => untrack(() => cleanNode(root))); @@ -236,6 +237,7 @@ export function createSignal( value, observers: null, observerSlots: null, + lastObserver: null, comparator: options.equals || undefined }; @@ -257,7 +259,10 @@ export function createSignal( return writeSignal(s, value); }; - return [readSignal.bind(s), setter]; + const result = [readSignal.bind(s), setter] as Signal; + if (IS_DEV) (result as any).state = s; + + return result; } export interface BaseOptions { @@ -269,7 +274,7 @@ export interface BaseOptions { // TypeScript Discord conversation: https://discord.com/channels/508357248330760243/508357248330760249/911266491024949328 export type NoInfer = [T][T extends any ? 0 : never]; -export interface EffectOptions extends BaseOptions {} +export interface EffectOptions extends BaseOptions { } // Also similar to OnEffectFunction export type EffectFunction = (v: Prev) => Next; @@ -386,15 +391,15 @@ export function createEffect( export function createReaction(onInvalidate: () => void, options?: EffectOptions) { let fn: (() => void) | undefined; const c = createComputation( - () => { - fn ? fn() : untrack(onInvalidate); - fn = undefined; - }, - undefined, - false, - 0, - IS_DEV ? options : undefined - ), + () => { + fn ? fn() : untrack(onInvalidate); + fn = undefined; + }, + undefined, + false, + 0, + IS_DEV ? options : undefined + ), s = SuspenseContext && useContext(SuspenseContext); if (s) c.suspense = s; c.user = true; @@ -700,15 +705,15 @@ export function createResource( initP !== NO_INIT ? (initP as T | Promise) : untrack(() => { - try { - return fetcher(lookup, { - value: value(), - refetching - }); - } catch (fetcherError) { - error = fetcherError; - } - }); + try { + return fetcher(lookup, { + value: value(), + refetching + }); + } catch (fetcherError) { + error = fetcherError; + } + }); if (error !== undefined) { loadEnd(pr, undefined, castError(error), lookup); return; @@ -909,8 +914,8 @@ export function untrack(fn: Accessor): T { export type ReturnTypes = T extends readonly Accessor[] ? { [K in keyof T]: T[K] extends Accessor ? I : never } : T extends Accessor - ? I - : never; + ? I + : never; // transforms a tuple to a tuple of accessors in a way that allows generics to be inferred export type AccessorArray = [...Extract<{ [K in keyof T]: Accessor }, readonly unknown[]>]; @@ -1305,7 +1310,8 @@ export function readSignal(this: SignalState | Memo) { Updates = updates; } } - if (Listener) { + if (Listener && this.lastObserver !== Listener) { + this.lastObserver = Listener; const sSlot = this.observers ? this.observers.length : 0; if (!Listener.sources) { Listener.sources = [this]; @@ -1691,6 +1697,7 @@ function cleanNode(node: Owner) { source.observerSlots![index] = s; } } + source.lastObserver = null; } } @@ -1771,10 +1778,10 @@ function createProvider(id: symbol, options?: EffectOptions) { let res; createRenderEffect( () => - (res = untrack(() => { - Owner!.context = { ...Owner!.context, [id]: props.value }; - return children(() => props.children); - })), + (res = untrack(() => { + Owner!.context = { ...Owner!.context, [id]: props.value }; + return children(() => props.children); + })), undefined, options ); diff --git a/packages/solid/test/signals.spec.ts b/packages/solid/test/signals.spec.ts index 8e23534bf..6434adc03 100644 --- a/packages/solid/test/signals.spec.ts +++ b/packages/solid/test/signals.spec.ts @@ -605,6 +605,25 @@ describe("catchError", () => { expect(errored).toBe(true); }); + describe("Repeatedly read the signal", () => { + test("Signal repeated read in for loop", () => { + const a = createSignal(0); + const dispose = createRoot(dispose => { + createEffect(() => { + for (let i = 0; i < 1000; i++) { + a[0](); + } + }); + return dispose; + }); + expect((a as any).state.observers.length).toBe(1); + expect((a as any).state.observerSlots.length).toBe(1); + expect((a as any).state.observers[0].sources.length).toBe(1); + expect((a as any).state.observers[0].sourceSlots.length).toBe(1); + dispose(); + }); +}); + test("In nested memo", () => { let errored = false; expect(() => From 0bb4dad0f619de5bcc1ba4ecec95311de42c80f2 Mon Sep 17 00:00:00 2001 From: Lin Tao <3520043790@qq.com> Date: Thu, 2 Apr 2026 17:29:02 +0800 Subject: [PATCH 2/2] formate code --- packages/solid/src/reactive/signal.ts | 66 +++++++++++++-------------- packages/solid/test/signals.spec.ts | 28 ++++++------ 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/packages/solid/src/reactive/signal.ts b/packages/solid/src/reactive/signal.ts index 713d2da28..4527ed328 100644 --- a/packages/solid/src/reactive/signal.ts +++ b/packages/solid/src/reactive/signal.ts @@ -158,17 +158,17 @@ export function createRoot(fn: RootFunction, detachedOwner?: typeof Owner) ? { owned: null, cleanups: null, context: null, owner: null } : UNOWNED : { - owned: null, - cleanups: null, - context: current ? current.context : null, - owner: current - }, + owned: null, + cleanups: null, + context: current ? current.context : null, + owner: current + }, updateFn = unowned ? IS_DEV ? () => - fn(() => { - throw new Error("Dispose method must be an explicit argument to createRoot function"); - }) + fn(() => { + throw new Error("Dispose method must be an explicit argument to createRoot function"); + }) : fn : () => fn(() => untrack(() => cleanNode(root))); @@ -274,7 +274,7 @@ export interface BaseOptions { // TypeScript Discord conversation: https://discord.com/channels/508357248330760243/508357248330760249/911266491024949328 export type NoInfer = [T][T extends any ? 0 : never]; -export interface EffectOptions extends BaseOptions { } +export interface EffectOptions extends BaseOptions {} // Also similar to OnEffectFunction export type EffectFunction = (v: Prev) => Next; @@ -391,15 +391,15 @@ export function createEffect( export function createReaction(onInvalidate: () => void, options?: EffectOptions) { let fn: (() => void) | undefined; const c = createComputation( - () => { - fn ? fn() : untrack(onInvalidate); - fn = undefined; - }, - undefined, - false, - 0, - IS_DEV ? options : undefined - ), + () => { + fn ? fn() : untrack(onInvalidate); + fn = undefined; + }, + undefined, + false, + 0, + IS_DEV ? options : undefined + ), s = SuspenseContext && useContext(SuspenseContext); if (s) c.suspense = s; c.user = true; @@ -705,15 +705,15 @@ export function createResource( initP !== NO_INIT ? (initP as T | Promise) : untrack(() => { - try { - return fetcher(lookup, { - value: value(), - refetching - }); - } catch (fetcherError) { - error = fetcherError; - } - }); + try { + return fetcher(lookup, { + value: value(), + refetching + }); + } catch (fetcherError) { + error = fetcherError; + } + }); if (error !== undefined) { loadEnd(pr, undefined, castError(error), lookup); return; @@ -914,8 +914,8 @@ export function untrack(fn: Accessor): T { export type ReturnTypes = T extends readonly Accessor[] ? { [K in keyof T]: T[K] extends Accessor ? I : never } : T extends Accessor - ? I - : never; + ? I + : never; // transforms a tuple to a tuple of accessors in a way that allows generics to be inferred export type AccessorArray = [...Extract<{ [K in keyof T]: Accessor }, readonly unknown[]>]; @@ -1778,10 +1778,10 @@ function createProvider(id: symbol, options?: EffectOptions) { let res; createRenderEffect( () => - (res = untrack(() => { - Owner!.context = { ...Owner!.context, [id]: props.value }; - return children(() => props.children); - })), + (res = untrack(() => { + Owner!.context = { ...Owner!.context, [id]: props.value }; + return children(() => props.children); + })), undefined, options ); diff --git a/packages/solid/test/signals.spec.ts b/packages/solid/test/signals.spec.ts index 6434adc03..9dcf1e1d8 100644 --- a/packages/solid/test/signals.spec.ts +++ b/packages/solid/test/signals.spec.ts @@ -606,23 +606,23 @@ describe("catchError", () => { }); describe("Repeatedly read the signal", () => { - test("Signal repeated read in for loop", () => { - const a = createSignal(0); - const dispose = createRoot(dispose => { - createEffect(() => { - for (let i = 0; i < 1000; i++) { - a[0](); - } + test("Signal repeated read in for loop", () => { + const a = createSignal(0); + const dispose = createRoot(dispose => { + createEffect(() => { + for (let i = 0; i < 1000; i++) { + a[0](); + } + }); + return dispose; }); - return dispose; + expect((a as any).state.observers.length).toBe(1); + expect((a as any).state.observerSlots.length).toBe(1); + expect((a as any).state.observers[0].sources.length).toBe(1); + expect((a as any).state.observers[0].sourceSlots.length).toBe(1); + dispose(); }); - expect((a as any).state.observers.length).toBe(1); - expect((a as any).state.observerSlots.length).toBe(1); - expect((a as any).state.observers[0].sources.length).toBe(1); - expect((a as any).state.observers[0].sourceSlots.length).toBe(1); - dispose(); }); -}); test("In nested memo", () => { let errored = false;