Skip to content

Commit

Permalink
feat(signals): throw error in dev mode on state mutation (#4526)
Browse files Browse the repository at this point in the history
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;
});
  • Loading branch information
rainerhahnekamp authored Nov 18, 2024
1 parent ab1d1b4 commit 7a84209
Show file tree
Hide file tree
Showing 5 changed files with 179 additions and 9 deletions.
115 changes: 115 additions & 0 deletions modules/signals/spec/deep-freeze.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import { getState, patchState } from '../src/state-source';
import { signalState } from '../src/signal-state';
import { signalStore } from '../src/signal-store';
import { TestBed } from '@angular/core/testing';
import { withState } from '../src/with-state';

describe('deepFreeze', () => {
const initialState = {
user: {
firstName: 'John',
lastName: 'Smith',
},
foo: 'bar',
numbers: [1, 2, 3],
ngrx: 'signals',
};

for (const { stateFactory, name } of [
{
name: 'signalStore',
stateFactory: () => {
const Store = signalStore(
{ protectedState: false },
withState(initialState)
);
return TestBed.configureTestingModule({ providers: [Store] }).inject(
Store
);
},
},
{ name: 'signalState', stateFactory: () => signalState(initialState) },
]) {
describe(name, () => {
it(`throws on a mutable change`, () => {
const state = stateFactory();
expect(() =>
patchState(state, (state) => {
state.ngrx = 'mutable change';
return state;
})
).toThrowError("Cannot assign to read only property 'ngrx' of object");
});

it('throws on a nested mutable change', () => {
const state = stateFactory();
expect(() =>
patchState(state, (state) => {
state.user.firstName = 'mutable change';
return state;
})
).toThrowError(
"Cannot assign to read only property 'firstName' of object"
);
});
describe('mutable changes outside of patchState', () => {
it('throws on reassigned a property of the exposed state', () => {
const state = stateFactory();
expect(() => {
state.user().firstName = 'mutable change 1';
}).toThrowError(
"Cannot assign to read only property 'firstName' of object"
);
});

it('throws when exposed state via getState is mutated', () => {
const state = stateFactory();
const s = getState(state);

expect(() => (s.ngrx = 'mutable change 2')).toThrowError(
"Cannot assign to read only property 'ngrx' of object"
);
});

it('throws when mutable change happens for', () => {
const state = stateFactory();
const s = { user: { firstName: 'M', lastName: 'S' } };
patchState(state, s);

expect(() => {
s.user.firstName = 'mutable change 3';
}).toThrowError(
"Cannot assign to read only property 'firstName' of object"
);
});
});
});
}

describe('special tests', () => {
for (const { name, mutationFn } of [
{
name: 'location',
mutationFn: (state: { location: { city: string } }) =>
(state.location.city = 'Paris'),
},
{
name: 'user',
mutationFn: (state: { user: { firstName: string } }) =>
(state.user.firstName = 'Jane'),
},
]) {
it(`throws on concatenated state (${name})`, () => {
const UserStore = signalStore(
{ providedIn: 'root' },
withState(initialState),
withState({ location: { city: 'London' } })
);
const store = TestBed.inject(UserStore);
const state = getState(store);

expect(() => mutationFn(state)).toThrowError();
});
}
});
});
49 changes: 49 additions & 0 deletions modules/signals/src/deep-freeze.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
declare const ngDevMode: boolean;

export function deepFreeze<T>(target: T): T {
Object.freeze(target);

const targetIsFunction = typeof target === 'function';

Object.getOwnPropertyNames(target).forEach((prop) => {
// Ignore Ivy properties, ref: https://github.com/ngrx/platform/issues/2109#issuecomment-582689060
if (prop.startsWith('ɵ')) {
return;
}

if (
hasOwnProperty(target, prop) &&
(targetIsFunction
? prop !== 'caller' && prop !== 'callee' && prop !== 'arguments'
: true)
) {
const propValue = target[prop];

if (
(isObjectLike(propValue) || typeof propValue === 'function') &&
!Object.isFrozen(propValue)
) {
deepFreeze(propValue);
}
}
});

return target;
}

export function freezeInDevMode<T>(target: T): T {
return ngDevMode ? deepFreeze(target) : target;
}

function hasOwnProperty(
target: unknown,
propertyName: string
): target is { [propertyName: string]: unknown } {
return isObjectLike(target)
? Object.prototype.hasOwnProperty.call(target, propertyName)
: false;
}

function isObjectLike(target: unknown): target is object {
return typeof target === 'object' && target !== null;
}
3 changes: 2 additions & 1 deletion modules/signals/src/signal-state.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import { signal } from '@angular/core';
import { STATE_SOURCE, WritableStateSource } from './state-source';
import { DeepSignal, toDeepSignal } from './deep-signal';
import { freezeInDevMode } from './deep-freeze';

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

export function signalState<State extends object>(
initialState: State
): SignalState<State> {
const stateSource = signal(initialState as State);
const stateSource = signal(freezeInDevMode(initialState as State));
const signalState = toDeepSignal(stateSource.asReadonly());
Object.defineProperty(signalState, STATE_SOURCE, {
value: stateSource,
Expand Down
10 changes: 6 additions & 4 deletions modules/signals/src/state-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
WritableSignal,
} from '@angular/core';
import { Prettify } from './ts-helpers';
import { freezeInDevMode } from './deep-freeze';

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

Expand Down Expand Up @@ -37,10 +38,11 @@ export function patchState<State extends object>(
): void {
stateSource[STATE_SOURCE].update((currentState) =>
updaters.reduce(
(nextState: State, updater) => ({
...nextState,
...(typeof updater === 'function' ? updater(nextState) : updater),
}),
(nextState: State, updater) =>
freezeInDevMode({
...nextState,
...(typeof updater === 'function' ? updater(nextState) : updater),
}),
currentState
)
);
Expand Down
11 changes: 7 additions & 4 deletions modules/signals/src/with-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
SignalStoreFeature,
SignalStoreFeatureResult,
} from './signal-store-models';
import { freezeInDevMode } from './deep-freeze';

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

assertUniqueStoreMembers(store, stateKeys);

store[STATE_SOURCE].update((currentState) => ({
...currentState,
...state,
}));
store[STATE_SOURCE].update((currentState) =>
freezeInDevMode({
...currentState,
...state,
})
);

const stateSignals = stateKeys.reduce((acc, key) => {
const sliceSignal = computed(
Expand Down

0 comments on commit 7a84209

Please sign in to comment.