diff --git a/packages/store/internals/src/memoize.ts b/packages/store/internals/src/memoize.ts index 503b991b8..ba7226fe1 100644 --- a/packages/store/internals/src/memoize.ts +++ b/packages/store/internals/src/memoize.ts @@ -1,8 +1,4 @@ -function areArgumentsShallowlyEqual( - equalityCheck: (a: any, b: any) => boolean, - prev: IArguments | null, - next: IArguments | null -) { +function areArgumentsShallowlyEqual(prev: IArguments | null, next: IArguments | null) { if (prev === null || next === null || prev.length !== next.length) { return false; } @@ -11,7 +7,7 @@ function areArgumentsShallowlyEqual( // determine equality as fast as possible. const length = prev.length; for (let i = 0; i < length; i++) { - if (!equalityCheck(prev[i], next[i])) { + if (!Object.is(prev[i], next[i])) { return false; } } @@ -25,28 +21,28 @@ function areArgumentsShallowlyEqual( * * @ignore */ -export function ɵmemoize any>( - func: T, - equalityCheck = Object.is -): T { +export function ɵmemoize any>(fn: T, originalFn?: T): T { let lastArgs: IArguments | null = null; let lastResult: any = null; // we reference arguments instead of spreading them for performance reasons function memoized() { // eslint-disable-next-line prefer-rest-params - if (!areArgumentsShallowlyEqual(equalityCheck, lastArgs, arguments)) { + if (!areArgumentsShallowlyEqual(lastArgs, arguments)) { // apply arguments instead of spreading for performance. // eslint-disable-next-line prefer-rest-params, prefer-spread - lastResult = (func).apply(null, arguments); + lastResult = (fn).apply(null, arguments); } // eslint-disable-next-line prefer-rest-params lastArgs = arguments; return lastResult; } - (memoized).reset = function () { - // The hidden (for now) ability to reset the memoization - lastArgs = null; - lastResult = null; - }; + if (typeof ngDevMode !== 'undefined' && ngDevMode) { + (memoized).reset = function () { + // The hidden (for now) ability to reset the memoization + lastArgs = null; + lastResult = null; + }; + (memoized).originalFn = originalFn ?? fn; + } return memoized as T; } diff --git a/packages/store/src/selectors/selector-utils.ts b/packages/store/src/selectors/selector-utils.ts index 53737a890..0a46a43c9 100644 --- a/packages/store/src/selectors/selector-utils.ts +++ b/packages/store/src/selectors/selector-utils.ts @@ -75,7 +75,7 @@ export function createMemoizedSelectorFn any>( } return returnValue; } as T; - const memoizedFn = ɵmemoize(wrappedFn); + const memoizedFn = ɵmemoize(wrappedFn, originalFn); Object.setPrototypeOf(memoizedFn, originalFn); return memoizedFn; } diff --git a/packages/store/src/store.ts b/packages/store/src/store.ts index 495e51a6a..ab8dfe3e7 100644 --- a/packages/store/src/store.ts +++ b/packages/store/src/store.ts @@ -122,10 +122,11 @@ export class Store { )?.warnOnNewReferenceWithIdenticalValue; if (warnOption && !Object.is(a, b)) { try { - if (warnOption.isEqual(a, b)) { + const originalFn = (selector).originalFn; + if (warnOption.isEqual(a, b) && originalFn) { console.error( 'The selector returned the same value shape as it was before triggering signal recomputation. Selector function: ', - selector + originalFn ); } } catch { diff --git a/packages/store/tests/select-signal.spec.ts b/packages/store/tests/select-signal.spec.ts index e0ba23f4c..41f91a32d 100644 --- a/packages/store/tests/select-signal.spec.ts +++ b/packages/store/tests/select-signal.spec.ts @@ -7,7 +7,9 @@ import { NgxsModule, Selector, SelectorOptions, - TypedSelector + TypedSelector, + provideStore, + withNgxsDevelopmentOptions } from '@ngxs/store'; import { ɵStateClass } from '@ngxs/store/internals'; @@ -792,3 +794,120 @@ describe('Selector', () => { }); }); }); + +describe('warnOnNewReferenceWithIdenticalValue', () => { + interface TestStateModel { + value: number; + counter: number; + } + + @State({ + name: 'testWarn', + defaults: { value: 1, counter: 0 } + }) + @Injectable() + class TestWarnState { + // Always returns a new object reference — same shape when `value` unchanged. + @Selector() + static data(state: TestStateModel) { + return { value: state.value }; + } + } + + beforeEach(() => TestBed.resetTestingModule()); + + it('should log console.error when selector returns new reference with identical value', () => { + // Arrange + TestBed.configureTestingModule({ + providers: [ + provideStore( + [TestWarnState], + withNgxsDevelopmentOptions({ + warnOnUnhandledActions: true, + warnOnNewReferenceWithIdenticalValue: { + isEqual: (a, b) => JSON.stringify(a) === JSON.stringify(b) + } + }) + ) + ] + }); + const store = TestBed.inject(Store); + const errorSpy = jest.spyOn(console, 'error').mockImplementation(); + + try { + const signal = TestBed.runInInjectionContext(() => + store.selectSignal(TestWarnState.data) + ); + signal(); // initial read + + // Increment `counter` but keep `value` the same — selector returns a new + // reference with the same shape, triggering the warning. + store.reset({ testWarn: { value: 1, counter: 1 } }); + signal(); // triggers recomputation and the equal() callback + + expect(errorSpy).toHaveBeenCalledWith( + 'The selector returned the same value shape as it was before triggering signal recomputation. Selector function: ', + expect.any(Function) + ); + } finally { + errorSpy.mockRestore(); + } + }); + + it('should not log console.error when warnOnNewReferenceWithIdenticalValue is not configured', () => { + // Arrange + TestBed.configureTestingModule({ + providers: [provideStore([TestWarnState])] + }); + const store = TestBed.inject(Store); + const errorSpy = jest.spyOn(console, 'error').mockImplementation(); + + try { + const signal = TestBed.runInInjectionContext(() => + store.selectSignal(TestWarnState.data) + ); + signal(); + + store.reset({ testWarn: { value: 1, counter: 1 } }); + signal(); + + expect(errorSpy).not.toHaveBeenCalled(); + } finally { + errorSpy.mockRestore(); + } + }); + + it('should not log console.error when selector returns a genuinely different value', () => { + // Arrange + TestBed.configureTestingModule({ + providers: [ + provideStore( + [TestWarnState], + withNgxsDevelopmentOptions({ + warnOnUnhandledActions: true, + warnOnNewReferenceWithIdenticalValue: { + isEqual: (a, b) => JSON.stringify(a) === JSON.stringify(b) + } + }) + ) + ] + }); + const store = TestBed.inject(Store); + const errorSpy = jest.spyOn(console, 'error').mockImplementation(); + + try { + const signal = TestBed.runInInjectionContext(() => + store.selectSignal(TestWarnState.data) + ); + signal(); + + // Change `value` — selector returns a genuinely different result. + store.reset({ testWarn: { value: 2, counter: 0 } }); + signal(); + + expect(errorSpy).not.toHaveBeenCalled(); + } finally { + errorSpy.mockRestore(); + } + }); +});