Skip to content

Commit 7a84209

Browse files
feat(signals): throw error in dev mode on state mutation (#4526)
BREAKING CHANGES: The `signalState`/`signalStore` state object is frozen in development mode. If a mutable change occurs to the state object, an error will be thrown. BEFORE: const userState = signalState(initialState); patchState(userState, (state) => { state.user.firstName = 'mutable change'; // mutable change which went through return state; }); AFTER: const userState = signalState(initialState); patchState(userState, (state) => { state.user.firstName = 'mutable change'; // throws in dev mode return state; });
1 parent ab1d1b4 commit 7a84209

File tree

5 files changed

+179
-9
lines changed

5 files changed

+179
-9
lines changed
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
import { getState, patchState } from '../src/state-source';
2+
import { signalState } from '../src/signal-state';
3+
import { signalStore } from '../src/signal-store';
4+
import { TestBed } from '@angular/core/testing';
5+
import { withState } from '../src/with-state';
6+
7+
describe('deepFreeze', () => {
8+
const initialState = {
9+
user: {
10+
firstName: 'John',
11+
lastName: 'Smith',
12+
},
13+
foo: 'bar',
14+
numbers: [1, 2, 3],
15+
ngrx: 'signals',
16+
};
17+
18+
for (const { stateFactory, name } of [
19+
{
20+
name: 'signalStore',
21+
stateFactory: () => {
22+
const Store = signalStore(
23+
{ protectedState: false },
24+
withState(initialState)
25+
);
26+
return TestBed.configureTestingModule({ providers: [Store] }).inject(
27+
Store
28+
);
29+
},
30+
},
31+
{ name: 'signalState', stateFactory: () => signalState(initialState) },
32+
]) {
33+
describe(name, () => {
34+
it(`throws on a mutable change`, () => {
35+
const state = stateFactory();
36+
expect(() =>
37+
patchState(state, (state) => {
38+
state.ngrx = 'mutable change';
39+
return state;
40+
})
41+
).toThrowError("Cannot assign to read only property 'ngrx' of object");
42+
});
43+
44+
it('throws on a nested mutable change', () => {
45+
const state = stateFactory();
46+
expect(() =>
47+
patchState(state, (state) => {
48+
state.user.firstName = 'mutable change';
49+
return state;
50+
})
51+
).toThrowError(
52+
"Cannot assign to read only property 'firstName' of object"
53+
);
54+
});
55+
describe('mutable changes outside of patchState', () => {
56+
it('throws on reassigned a property of the exposed state', () => {
57+
const state = stateFactory();
58+
expect(() => {
59+
state.user().firstName = 'mutable change 1';
60+
}).toThrowError(
61+
"Cannot assign to read only property 'firstName' of object"
62+
);
63+
});
64+
65+
it('throws when exposed state via getState is mutated', () => {
66+
const state = stateFactory();
67+
const s = getState(state);
68+
69+
expect(() => (s.ngrx = 'mutable change 2')).toThrowError(
70+
"Cannot assign to read only property 'ngrx' of object"
71+
);
72+
});
73+
74+
it('throws when mutable change happens for', () => {
75+
const state = stateFactory();
76+
const s = { user: { firstName: 'M', lastName: 'S' } };
77+
patchState(state, s);
78+
79+
expect(() => {
80+
s.user.firstName = 'mutable change 3';
81+
}).toThrowError(
82+
"Cannot assign to read only property 'firstName' of object"
83+
);
84+
});
85+
});
86+
});
87+
}
88+
89+
describe('special tests', () => {
90+
for (const { name, mutationFn } of [
91+
{
92+
name: 'location',
93+
mutationFn: (state: { location: { city: string } }) =>
94+
(state.location.city = 'Paris'),
95+
},
96+
{
97+
name: 'user',
98+
mutationFn: (state: { user: { firstName: string } }) =>
99+
(state.user.firstName = 'Jane'),
100+
},
101+
]) {
102+
it(`throws on concatenated state (${name})`, () => {
103+
const UserStore = signalStore(
104+
{ providedIn: 'root' },
105+
withState(initialState),
106+
withState({ location: { city: 'London' } })
107+
);
108+
const store = TestBed.inject(UserStore);
109+
const state = getState(store);
110+
111+
expect(() => mutationFn(state)).toThrowError();
112+
});
113+
}
114+
});
115+
});

modules/signals/src/deep-freeze.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
declare const ngDevMode: boolean;
2+
3+
export function deepFreeze<T>(target: T): T {
4+
Object.freeze(target);
5+
6+
const targetIsFunction = typeof target === 'function';
7+
8+
Object.getOwnPropertyNames(target).forEach((prop) => {
9+
// Ignore Ivy properties, ref: https://github.com/ngrx/platform/issues/2109#issuecomment-582689060
10+
if (prop.startsWith('ɵ')) {
11+
return;
12+
}
13+
14+
if (
15+
hasOwnProperty(target, prop) &&
16+
(targetIsFunction
17+
? prop !== 'caller' && prop !== 'callee' && prop !== 'arguments'
18+
: true)
19+
) {
20+
const propValue = target[prop];
21+
22+
if (
23+
(isObjectLike(propValue) || typeof propValue === 'function') &&
24+
!Object.isFrozen(propValue)
25+
) {
26+
deepFreeze(propValue);
27+
}
28+
}
29+
});
30+
31+
return target;
32+
}
33+
34+
export function freezeInDevMode<T>(target: T): T {
35+
return ngDevMode ? deepFreeze(target) : target;
36+
}
37+
38+
function hasOwnProperty(
39+
target: unknown,
40+
propertyName: string
41+
): target is { [propertyName: string]: unknown } {
42+
return isObjectLike(target)
43+
? Object.prototype.hasOwnProperty.call(target, propertyName)
44+
: false;
45+
}
46+
47+
function isObjectLike(target: unknown): target is object {
48+
return typeof target === 'object' && target !== null;
49+
}

modules/signals/src/signal-state.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import { signal } from '@angular/core';
22
import { STATE_SOURCE, WritableStateSource } from './state-source';
33
import { DeepSignal, toDeepSignal } from './deep-signal';
4+
import { freezeInDevMode } from './deep-freeze';
45

56
export type SignalState<State extends object> = DeepSignal<State> &
67
WritableStateSource<State>;
78

89
export function signalState<State extends object>(
910
initialState: State
1011
): SignalState<State> {
11-
const stateSource = signal(initialState as State);
12+
const stateSource = signal(freezeInDevMode(initialState as State));
1213
const signalState = toDeepSignal(stateSource.asReadonly());
1314
Object.defineProperty(signalState, STATE_SOURCE, {
1415
value: stateSource,

modules/signals/src/state-source.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
WritableSignal,
99
} from '@angular/core';
1010
import { Prettify } from './ts-helpers';
11+
import { freezeInDevMode } from './deep-freeze';
1112

1213
const STATE_WATCHERS = new WeakMap<Signal<object>, Array<StateWatcher<any>>>();
1314

@@ -37,10 +38,11 @@ export function patchState<State extends object>(
3738
): void {
3839
stateSource[STATE_SOURCE].update((currentState) =>
3940
updaters.reduce(
40-
(nextState: State, updater) => ({
41-
...nextState,
42-
...(typeof updater === 'function' ? updater(nextState) : updater),
43-
}),
41+
(nextState: State, updater) =>
42+
freezeInDevMode({
43+
...nextState,
44+
...(typeof updater === 'function' ? updater(nextState) : updater),
45+
}),
4446
currentState
4547
)
4648
);

modules/signals/src/with-state.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
SignalStoreFeature,
1010
SignalStoreFeatureResult,
1111
} from './signal-store-models';
12+
import { freezeInDevMode } from './deep-freeze';
1213

1314
export function withState<State extends object>(
1415
stateFactory: () => State
@@ -35,10 +36,12 @@ export function withState<State extends object>(
3536

3637
assertUniqueStoreMembers(store, stateKeys);
3738

38-
store[STATE_SOURCE].update((currentState) => ({
39-
...currentState,
40-
...state,
41-
}));
39+
store[STATE_SOURCE].update((currentState) =>
40+
freezeInDevMode({
41+
...currentState,
42+
...state,
43+
})
44+
);
4245

4346
const stateSignals = stateKeys.reduce((acc, key) => {
4447
const sliceSignal = computed(

0 commit comments

Comments
 (0)