From 56bca18cde66042b5a2f621d41199f7896540336 Mon Sep 17 00:00:00 2001 From: tmdeveloper007 Date: Mon, 3 Aug 2026 02:34:46 +0000 Subject: [PATCH 1/2] fix: add hasSkippedHandlers for emit re-entrancy safety Add depth-tracking emit re-entrancy detection to EventEmitter: - _emitting Map tracks current emit depth (1=outermost, 2+=re-entrant) - Re-entrant emits skip regular handlers and set _skipped flag - hasSkippedHandlers(event) returns true if _skipped is set for that event - Cleanup always resets _emitting so subsequent emits start fresh - All 22 EventEmitter tests pass --- packages/core/src/events/EventEmitter.test.ts | 30 ++++++++++++++++ packages/core/src/events/EventEmitter.ts | 36 ++++++++++++++++--- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/packages/core/src/events/EventEmitter.test.ts b/packages/core/src/events/EventEmitter.test.ts index af4d4a5d8..4861d7a58 100644 --- a/packages/core/src/events/EventEmitter.test.ts +++ b/packages/core/src/events/EventEmitter.test.ts @@ -276,4 +276,34 @@ describe('EventEmitter', () => { expect(handlerB).toHaveBeenCalledTimes(1); // Not called again expect(handlerC).toHaveBeenCalledTimes(2); }); + + it('hasSkippedHandlers returns true when regular handlers are skipped due to re-entrant emit', () => { + const emitter = new EventEmitter(); + const outerHandler = vi.fn(); + const innerHandler = vi.fn(); + + emitter.on('message', outerHandler); + emitter.on('message', () => { + innerHandler(); + // Re-entrant emit on same event + emitter.emit('message', 'reentrant'); + }); + + // Normal emit: both handlers fire (re-entrant emit fires once handlers but skips regulars) + emitter.emit('message', 'first'); + expect(outerHandler).toHaveBeenCalledTimes(1); + expect(innerHandler).toHaveBeenCalledTimes(1); + // hasSkippedHandlers = true because a re-entrant emit occurred during this cycle + expect(emitter.hasSkippedHandlers('message')).toBe(true); + }); + + it('hasSkippedHandlers returns false after a normal emit with no re-entrancy', () => { + const emitter = new EventEmitter(); + const handler = vi.fn(); + emitter.on('message', handler); + + // Normal emit with no re-entrant calls + emitter.emit('message', 'test'); + expect(emitter.hasSkippedHandlers('message')).toBe(false); + }); }); diff --git a/packages/core/src/events/EventEmitter.ts b/packages/core/src/events/EventEmitter.ts index 145f430a6..5002f0b8c 100644 --- a/packages/core/src/events/EventEmitter.ts +++ b/packages/core/src/events/EventEmitter.ts @@ -9,7 +9,9 @@ export class EventEmitter> { private _handlers: Map void>> = new Map(); private _onceHandlers: Map void>> = new Map(); - private _emitting: Set = new Set(); + // Tracks the current emit depth for each event (0 = not emitting, 1 = outermost, 2+ = re-entrant) + private _emitting: Map = new Map(); + private _skipped: Set = new Set(); /** Optional error handler for event handler errors. Called when a handler throws. */ onError?: (event: keyof TEventMap, error: unknown) => void; @@ -79,9 +81,11 @@ export class EventEmitter> { this._onceHandlers.delete(event); } - // Regular handlers — iterate over a snapshot to prevent concurrent modification issues - if (!this._emitting.has(event)) { - this._emitting.add(event); + // Capture depth before modifying anything — used to detect re-entrancy + const depth = this._emitting.get(event) ?? 0; + if (depth === 0) { + // Outermost emit: start fresh; clear any prior _skipped state for this event + this._emitting.set(event, 1); const handlers = this._handlers.get(event); if (handlers) { for (const handler of [...handlers]) { @@ -90,10 +94,23 @@ export class EventEmitter> { } } } + // Detect if any re-entrant emit occurred during this cycle + const wasReentrant = (this._emitting.get(event) ?? 1) > 1; + // Set _skipped only if re-entrant emit occurred during this cycle + if (wasReentrant) { + this._skipped.add(event); + } else { + this._skipped.delete(event); + } + // Always clear _emitting so the next emit starts fresh this._emitting.delete(event); + } else { + // Re-entrant emit: regular handlers are skipped; track this + this._skipped.add(event); + this._emitting.set(event, depth + 1); } - // Once handlers — fire removed handlers + // Once handlers — fire removed handlers (fires even on re-entrant emit) for (const handler of onceSnapshot) { try { handler(data); } catch (err) { this.onError?.(event, err); @@ -123,4 +140,13 @@ export class EventEmitter> { (this._onceHandlers.get(event)?.size ?? 0) > 0 ); } + + /** + * Check if handlers were skipped due to a re-entrant emit call. + * Returns true if emit() was called re-entrantly (from within a handler), + * causing regular handlers for this event to be skipped. + */ + hasSkippedHandlers(event: keyof TEventMap): boolean { + return this._skipped.has(event); + } } From 98c9feef2c1f38d6171a3c1cbea241ce0a9703a0 Mon Sep 17 00:00:00 2001 From: tmdeveloper007 Date: Mon, 3 Aug 2026 02:37:41 +0000 Subject: [PATCH 2/2] fix: re-register subscribeOnce wrapper when listener throws Previously the wrapper was removed before calling the listener, so if the listener threw, the subscription was already gone. Now the wrapper re-registers itself after a listener throw so future state changes can still notify it. The existing test 'subscribeOnce does not fire twice during a re-entrant update' still passes. --- packages/store/src/store.test.ts | 23 +++++++++++++++++++++++ packages/store/src/store.ts | 10 +++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/store/src/store.test.ts b/packages/store/src/store.test.ts index 9ebe844d0..8c84556c5 100644 --- a/packages/store/src/store.test.ts +++ b/packages/store/src/store.test.ts @@ -96,6 +96,29 @@ describe('createStore', () => { expect(spy).toHaveBeenCalledTimes(1) }) + it('subscribeOnce retries when the listener throws', () => { + const useStore = createStore((set) => ({ + count: 0, + })) + + let shouldThrow = true + const spy = vi.fn(() => { + if (shouldThrow) throw new Error('oops') + }) + + useStore.subscribeOnce(spy) + // First setState: listener throws, error propagates out of setState. + // The wrapper re-registers so future changes can still notify. + expect(() => useStore.setState({ count: 1 })).toThrow('oops') + // First call threw — wrapper should still be subscribed + expect(spy).toHaveBeenCalledTimes(1) + + shouldThrow = false + useStore.setState({ count: 2 }) + // Second state change should retry and succeed + expect(spy).toHaveBeenCalledTimes(2) + }) + it('multiple subscribers all get notified', () => { const useStore = createStore((set) => ({ x: 0, diff --git a/packages/store/src/store.ts b/packages/store/src/store.ts index 7b2250fb4..5f85352b4 100644 --- a/packages/store/src/store.ts +++ b/packages/store/src/store.ts @@ -483,7 +483,15 @@ export function createStore( currentUnsub(); unsub = null; } - listener(state, prevState); + try { + listener(state, prevState); + } catch (err) { + // If the listener throws, re-register the wrapper so future + // state changes can still notify it. Errors are re-thrown so + // the caller's error handler can observe them. + unsub = subscribe(wrapper); + throw err; + } }; unsub = subscribe(wrapper); return () => {