Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletions packages/store/internals/src/memoize.ts
Original file line number Diff line number Diff line change
@@ -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;
}
Expand All @@ -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;
}
}
Expand All @@ -25,28 +21,28 @@ function areArgumentsShallowlyEqual(
*
* @ignore
*/
export function ɵmemoize<T extends (...args: any[]) => any>(
func: T,
equalityCheck = Object.is
): T {
export function ɵmemoize<T extends (...args: any[]) => 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 = (<Function>func).apply(null, arguments);
lastResult = (<Function>fn).apply(null, arguments);
}
// eslint-disable-next-line prefer-rest-params
lastArgs = arguments;
return lastResult;
}
(<any>memoized).reset = function () {
// The hidden (for now) ability to reset the memoization
lastArgs = null;
lastResult = null;
};
if (typeof ngDevMode !== 'undefined' && ngDevMode) {
(<any>memoized).reset = function () {
// The hidden (for now) ability to reset the memoization
lastArgs = null;
lastResult = null;
};
(<any>memoized).originalFn = originalFn ?? fn;
}
return memoized as T;
}
2 changes: 1 addition & 1 deletion packages/store/src/selectors/selector-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function createMemoizedSelectorFn<T extends (...args: any[]) => any>(
}
return returnValue;
} as T;
const memoizedFn = ɵmemoize(wrappedFn);
const memoizedFn = ɵmemoize(wrappedFn, originalFn);
Object.setPrototypeOf(memoizedFn, originalFn);
return memoizedFn;
}
Expand Down
5 changes: 3 additions & 2 deletions packages/store/src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,11 @@ export class Store {
)?.warnOnNewReferenceWithIdenticalValue;
if (warnOption && !Object.is(a, b)) {
try {
if (warnOption.isEqual(a, b)) {
const originalFn = (<any>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 {
Expand Down
121 changes: 120 additions & 1 deletion packages/store/tests/select-signal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import {
NgxsModule,
Selector,
SelectorOptions,
TypedSelector
TypedSelector,
provideStore,
withNgxsDevelopmentOptions
} from '@ngxs/store';
import { ɵStateClass } from '@ngxs/store/internals';

Expand Down Expand Up @@ -792,3 +794,120 @@ describe('Selector', () => {
});
});
});

describe('warnOnNewReferenceWithIdenticalValue', () => {
interface TestStateModel {
value: number;
counter: number;
}

@State<TestStateModel>({
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();
}
});
});
Loading