From 2cf0c831c3e8232032120cac3c66bac2068b2269 Mon Sep 17 00:00:00 2001 From: Rainer Hahnekamp Date: Sat, 20 Sep 2025 01:06:09 +0200 Subject: [PATCH] fix(signals): expose readonly signals in slices `withState`, `signalState`, and `withLinkedState` now pass a `Signal` instead of the original `WritableSignal` to the DeepSignal. This prevents accidental misuse where consumers (e.g. `ngModel` or other APIs with incompatible types) could assert `WritableSignal` and write directly into the state. Closes #4958 --- modules/signals/spec/signal-state.spec.ts | 7 +++++++ modules/signals/spec/with-linked-state.spec.ts | 13 ++++++++++++- modules/signals/spec/with-state.spec.ts | 15 ++++++++++++++- modules/signals/src/signal-state.ts | 6 +++--- modules/signals/src/signal-store-models.ts | 7 ++++++- modules/signals/src/with-linked-state.ts | 5 +++-- modules/signals/src/with-state.ts | 10 ++++------ 7 files changed, 49 insertions(+), 14 deletions(-) diff --git a/modules/signals/spec/signal-state.spec.ts b/modules/signals/spec/signal-state.spec.ts index 000d63b015..4b3712b0bc 100644 --- a/modules/signals/spec/signal-state.spec.ts +++ b/modules/signals/spec/signal-state.spec.ts @@ -60,6 +60,13 @@ describe('signalState', () => { expect(isSignal(state.ngrx)).toBe(true); }); + it('does not expose state slices as writable signals', () => { + const state = signalState(initialState); + expect(() => (state as any).foo.set('baz')).toThrow( + 'set is not a function' + ); + }); + it('caches previously created signals', () => { const state = signalState(initialState); const user1 = state.user; diff --git a/modules/signals/spec/with-linked-state.spec.ts b/modules/signals/spec/with-linked-state.spec.ts index 12bd1f5992..38600418c1 100644 --- a/modules/signals/spec/with-linked-state.spec.ts +++ b/modules/signals/spec/with-linked-state.spec.ts @@ -1,4 +1,4 @@ -import { linkedSignal, signal } from '@angular/core'; +import { linkedSignal, signal, WritableSignal } from '@angular/core'; import { getState, patchState, @@ -254,4 +254,15 @@ describe('withLinkedState', () => { 'value' ); }); + + it('does not expose linked state properties as writable signals', () => { + const initialStore = getInitialInnerStore(); + const userStore = withLinkedState(() => ({ + foo: () => 'bar', + }))(initialStore); + + expect(() => + (userStore.stateSignals.foo as WritableSignal).set('baz') + ).toThrow('set is not a function'); + }); }); diff --git a/modules/signals/spec/with-state.spec.ts b/modules/signals/spec/with-state.spec.ts index 29b2b55e6c..7ffebdf136 100644 --- a/modules/signals/spec/with-state.spec.ts +++ b/modules/signals/spec/with-state.spec.ts @@ -1,4 +1,4 @@ -import { isSignal, signal } from '@angular/core'; +import { isSignal, signal, WritableSignal } from '@angular/core'; import { getState, withComputed, withMethods, withState } from '../src'; import { getInitialInnerStore } from '../src/signal-store'; @@ -38,6 +38,19 @@ describe('withState', () => { expect(isSignal(store.stateSignals.x.y)).toBe(true); }); + it('does not expose state slices as writable signals', () => { + const initialStore = getInitialInnerStore(); + + const store = withState({ + foo: 'bar', + x: { y: 'z' }, + })(initialStore); + + expect(() => + (store.stateSignals.foo as WritableSignal).set('baz') + ).toThrow('set is not a function'); + }); + it('patches state source and creates deep signals for state slices provided via factory', () => { const initialStore = getInitialInnerStore(); diff --git a/modules/signals/src/signal-state.ts b/modules/signals/src/signal-state.ts index 16f8211530..be6de736ab 100644 --- a/modules/signals/src/signal-state.ts +++ b/modules/signals/src/signal-state.ts @@ -1,6 +1,6 @@ import { computed, signal } from '@angular/core'; import { DeepSignal, toDeepSignal } from './deep-signal'; -import { SignalsDictionary } from './signal-store-models'; +import { WritableSignalsDictionary } from './signal-store-models'; import { STATE_SOURCE, WritableStateSource } from './state-source'; export type SignalState = DeepSignal & @@ -16,7 +16,7 @@ export function signalState( ...signalsDict, [key]: signal((initialState as Record)[key]), }), - {} as SignalsDictionary + {} as WritableSignalsDictionary ); const signalState = computed(() => @@ -32,7 +32,7 @@ export function signalState( for (const key of stateKeys) { Object.defineProperty(signalState, key, { - value: toDeepSignal(stateSource[key]), + value: toDeepSignal(stateSource[key].asReadonly()), }); } diff --git a/modules/signals/src/signal-store-models.ts b/modules/signals/src/signal-store-models.ts index e4c5779ccd..53d7fda056 100644 --- a/modules/signals/src/signal-store-models.ts +++ b/modules/signals/src/signal-store-models.ts @@ -1,4 +1,4 @@ -import { Signal } from '@angular/core'; +import { Signal, WritableSignal } from '@angular/core'; import { DeepSignal } from './deep-signal'; import { WritableStateSource } from './state-source'; import { IsKnownRecord, Prettify } from './ts-helpers'; @@ -14,6 +14,11 @@ export type StateSignals = export type SignalsDictionary = Record>; +export type WritableSignalsDictionary = Record< + string | symbol, + WritableSignal +>; + export type MethodsDictionary = Record; export type SignalStoreHooks = { diff --git a/modules/signals/src/with-linked-state.ts b/modules/signals/src/with-linked-state.ts index 6711250ff1..50b5d73b7c 100644 --- a/modules/signals/src/with-linked-state.ts +++ b/modules/signals/src/with-linked-state.ts @@ -7,6 +7,7 @@ import { SignalStoreFeature, SignalStoreFeatureResult, StateSignals, + WritableSignalsDictionary, } from './signal-store-models'; import { isWritableSignal, STATE_SOURCE } from './state-source'; import { Prettify } from './ts-helpers'; @@ -89,7 +90,7 @@ export function withLinkedState< if (typeof ngDevMode !== 'undefined' && ngDevMode) { assertUniqueStoreMembers(store, stateKeys); } - const stateSource = store[STATE_SOURCE] as SignalsDictionary; + const stateSource = store[STATE_SOURCE] as WritableSignalsDictionary; const stateSignals = {} as SignalsDictionary; for (const key of stateKeys) { @@ -97,7 +98,7 @@ export function withLinkedState< stateSource[key] = isWritableSignal(signalOrComputationFn) ? signalOrComputationFn : linkedSignal(signalOrComputationFn); - stateSignals[key] = toDeepSignal(stateSource[key]); + stateSignals[key] = toDeepSignal(stateSource[key].asReadonly()); } return { diff --git a/modules/signals/src/with-state.ts b/modules/signals/src/with-state.ts index aa46350b14..97a212b40b 100644 --- a/modules/signals/src/with-state.ts +++ b/modules/signals/src/with-state.ts @@ -1,4 +1,4 @@ -import { Signal, signal } from '@angular/core'; +import { signal } from '@angular/core'; import { toDeepSignal } from './deep-signal'; import { assertUniqueStoreMembers } from './signal-store-assertions'; import { @@ -7,6 +7,7 @@ import { SignalsDictionary, SignalStoreFeature, SignalStoreFeatureResult, + WritableSignalsDictionary, } from './signal-store-models'; import { STATE_SOURCE } from './state-source'; @@ -38,15 +39,12 @@ export function withState( assertUniqueStoreMembers(store, stateKeys); } - const stateSource = store[STATE_SOURCE] as Record< - string | symbol, - Signal - >; + const stateSource = store[STATE_SOURCE] as WritableSignalsDictionary; const stateSignals: SignalsDictionary = {}; for (const key of stateKeys) { stateSource[key] = signal(state[key]); - stateSignals[key] = toDeepSignal(stateSource[key]); + stateSignals[key] = toDeepSignal(stateSource[key].asReadonly()); } return {