diff --git a/modules/signals/spec/deep-freeze.spec.ts b/modules/signals/spec/deep-freeze.spec.ts index a432c94e25..4c0e420edf 100644 --- a/modules/signals/spec/deep-freeze.spec.ts +++ b/modules/signals/spec/deep-freeze.spec.ts @@ -1,10 +1,15 @@ -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'; +import { + getState, + patchState, + signalState, + signalStore, + withState, +} from '../src'; describe('deepFreeze', () => { + const SECRET = Symbol('secret'); + const initialState = { user: { firstName: 'John', @@ -13,6 +18,13 @@ describe('deepFreeze', () => { foo: 'bar', numbers: [1, 2, 3], ngrx: 'signals', + nestedSymbol: { + [SECRET]: 'another secret', + }, + [SECRET]: { + code: 'secret', + value: '123', + }, }; for (const { stateFactory, name } of [ @@ -52,6 +64,7 @@ describe('deepFreeze', () => { "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(); @@ -71,7 +84,7 @@ describe('deepFreeze', () => { ); }); - it('throws when mutable change happens for', () => { + it('throws when mutable change happens', () => { const state = stateFactory(); const s = { user: { firstName: 'M', lastName: 'S' } }; patchState(state, s); @@ -82,6 +95,28 @@ describe('deepFreeze', () => { "Cannot assign to read only property 'firstName' of object" ); }); + + it('throws when mutable change of root symbol property happens', () => { + const state = stateFactory(); + const s = getState(state); + + expect(() => { + s[SECRET].code = 'mutable change'; + }).toThrowError( + "Cannot assign to read only property 'code' of object" + ); + }); + + it('throws when mutable change of nested symbol property happens', () => { + const state = stateFactory(); + const s = getState(state); + + expect(() => { + s.nestedSymbol[SECRET] = 'mutable change'; + }).toThrowError( + "Cannot assign to read only property 'Symbol(secret)' of object" + ); + }); }); }); } diff --git a/modules/signals/spec/signal-store.spec.ts b/modules/signals/spec/signal-store.spec.ts index 4d388478b9..837e76527a 100644 --- a/modules/signals/spec/signal-store.spec.ts +++ b/modules/signals/spec/signal-store.spec.ts @@ -1,4 +1,10 @@ -import { inject, InjectionToken, isSignal, signal } from '@angular/core'; +import { + computed, + inject, + InjectionToken, + isSignal, + signal, +} from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { patchState, @@ -145,6 +151,14 @@ describe('signalStore', () => { expect(store.foo()).toBe('foo'); }); + + it('allows symbols as state keys', () => { + const SECRET = Symbol('SECRET'); + const Store = signalStore(withState({ [SECRET]: 'bar' })); + const store = new Store(); + + expect(store[SECRET]()).toBe('bar'); + }); }); describe('withProps', () => { @@ -183,6 +197,26 @@ describe('signalStore', () => { expect(store.foo).toBe('bar'); }); + + it('allows symbols as property keys', () => { + const SECRET = Symbol('SECRET'); + + const Store = signalStore(withProps(() => ({ [SECRET]: 'secret' }))); + const store = TestBed.configureTestingModule({ + providers: [Store], + }).inject(Store); + + expect(store[SECRET]).toBe('secret'); + }); + + it('allows numbers as property keys', () => { + const Store = signalStore(withProps(() => ({ 1: 'Number One' }))); + const store = TestBed.configureTestingModule({ + providers: [Store], + }).inject(Store); + + expect(store[1]).toBe('Number One'); + }); }); describe('withComputed', () => { @@ -221,6 +255,20 @@ describe('signalStore', () => { expect(store.bar()).toBe('bar'); }); + + it('allows symbols as computed keys', () => { + const SECRET = Symbol('SECRET'); + const SecretStore = signalStore( + { providedIn: 'root' }, + withComputed(() => ({ + [SECRET]: computed(() => 'secret'), + })) + ); + + const secretStore = TestBed.inject(SecretStore); + + expect(secretStore[SECRET]()).toBe('secret'); + }); }); describe('withMethods', () => { @@ -263,6 +311,19 @@ describe('signalStore', () => { expect(store.baz()).toBe('baz'); }); + + it('allows symbols as method keys', () => { + const SECRET = Symbol('SECRET'); + const SecretStore = signalStore( + { providedIn: 'root' }, + withMethods(() => ({ + [SECRET]: () => 'my secret', + })) + ); + const secretStore = TestBed.inject(SecretStore); + + expect(secretStore[SECRET]()).toBe('my secret'); + }); }); describe('withHooks', () => { @@ -455,5 +516,28 @@ describe('signalStore', () => { ], ]); }); + + it('passes on a symbol key to the features', () => { + const SECRET = Symbol('SECRET'); + const SecretStore = signalStore( + withProps(() => ({ + [SECRET]: 'not your business', + })), + withComputed((store) => ({ + secret: computed(() => store[SECRET]), + })), + withMethods((store) => ({ + reveil() { + return store[SECRET]; + }, + })) + ); + + const secretStore = new SecretStore(); + + expect(secretStore.reveil()).toBe('not your business'); + expect(secretStore.secret()).toBe('not your business'); + expect(secretStore[SECRET]).toBe('not your business'); + }); }); }); diff --git a/modules/signals/spec/state-source.spec.ts b/modules/signals/spec/state-source.spec.ts index 7da0d8b305..0f973ce238 100644 --- a/modules/signals/spec/state-source.spec.ts +++ b/modules/signals/spec/state-source.spec.ts @@ -18,6 +18,8 @@ import { import { STATE_SOURCE } from '../src/state-source'; import { createLocalService } from './helpers'; +const SECRET = Symbol('SECRET'); + describe('StateSource', () => { const initialState = { user: { @@ -27,6 +29,7 @@ describe('StateSource', () => { foo: 'bar', numbers: [1, 2, 3], ngrx: 'signals', + [SECRET]: 'secret', }; describe('patchState', () => { @@ -78,6 +81,13 @@ describe('StateSource', () => { }); }); + it('patches state slice with symbol key', () => { + const state = stateFactory(); + + patchState(state, { [SECRET]: 'another secret' }); + expect(state[SECRET]()).toBe('another secret'); + }); + it('patches state via sequence of partial state objects and updater functions', () => { const state = stateFactory(); diff --git a/modules/signals/spec/with-computed.spec.ts b/modules/signals/spec/with-computed.spec.ts index 38de74372b..e456005038 100644 --- a/modules/signals/spec/with-computed.spec.ts +++ b/modules/signals/spec/with-computed.spec.ts @@ -19,6 +19,7 @@ describe('withComputed', () => { }); it('logs warning if previously defined signal store members have the same name', () => { + const COMPUTED_SECRET = Symbol('computed_secret'); const initialStore = [ withState({ p1: 10, @@ -27,6 +28,7 @@ describe('withComputed', () => { withComputed(() => ({ s1: signal('s1').asReadonly(), s2: signal({ s: 2 }).asReadonly(), + [COMPUTED_SECRET]: signal(1).asReadonly(), })), withMethods(() => ({ m1() {}, @@ -43,12 +45,13 @@ describe('withComputed', () => { m1: signal({ m: 1 }).asReadonly(), m3: signal({ m: 3 }).asReadonly(), s3: signal({ s: 3 }).asReadonly(), + [COMPUTED_SECRET]: signal(10).asReadonly(), }))(initialStore); expect(console.warn).toHaveBeenCalledWith( '@ngrx/signals: SignalStore members cannot be overridden.', 'Trying to override:', - 'p1, s2, m1' + 'p1, s2, m1, Symbol(computed_secret)' ); }); }); diff --git a/modules/signals/spec/with-methods.spec.ts b/modules/signals/spec/with-methods.spec.ts index 1fdb03492b..16ee8ab7dc 100644 --- a/modules/signals/spec/with-methods.spec.ts +++ b/modules/signals/spec/with-methods.spec.ts @@ -19,14 +19,18 @@ describe('withMethods', () => { }); it('logs warning if previously defined signal store members have the same name', () => { + const STATE_SECRET = Symbol('state_secret'); + const COMPUTED_SECRET = Symbol('computed_secret'); const initialStore = [ withState({ p1: 'p1', p2: false, + [STATE_SECRET]: 1, }), withComputed(() => ({ s1: signal(true).asReadonly(), s2: signal({ s: 2 }).asReadonly(), + [COMPUTED_SECRET]: signal(1).asReadonly(), })), withMethods(() => ({ m1() {}, @@ -43,12 +47,14 @@ describe('withMethods', () => { s1: () => 100, m2, m3: () => 'm3', + [STATE_SECRET]() {}, + [COMPUTED_SECRET]() {}, }))(initialStore); expect(console.warn).toHaveBeenCalledWith( '@ngrx/signals: SignalStore members cannot be overridden.', 'Trying to override:', - 'p2, s1, m2' + 'p2, s1, m2, Symbol(state_secret), Symbol(computed_secret)' ); }); }); diff --git a/modules/signals/spec/with-props.spec.ts b/modules/signals/spec/with-props.spec.ts index ce473bb1c4..8ccdfd9baf 100644 --- a/modules/signals/spec/with-props.spec.ts +++ b/modules/signals/spec/with-props.spec.ts @@ -17,10 +17,13 @@ describe('withProps', () => { }); it('logs warning if previously defined signal store members have the same name', () => { + const STATE_SECRET = Symbol('state_secret'); + const METHOD_SECRET = Symbol('method_secret'); const initialStore = [ withState({ s1: 10, s2: 's2', + [STATE_SECRET]: 1, }), withProps(() => ({ p1: of(100), @@ -29,6 +32,7 @@ describe('withProps', () => { withMethods(() => ({ m1() {}, m2() {}, + [METHOD_SECRET]() {}, })), ].reduce((acc, feature) => feature(acc), getInitialInnerStore()); vi.spyOn(console, 'warn').mockImplementation(); @@ -39,12 +43,14 @@ describe('withProps', () => { p2: signal(100), m1: { ngrx: 'rocks' }, m3: of('m3'), + [STATE_SECRET]: 10, + [METHOD_SECRET]: { x: 'y' }, }))(initialStore); expect(console.warn).toHaveBeenCalledWith( '@ngrx/signals: SignalStore members cannot be overridden.', 'Trying to override:', - 's1, p2, m1' + 's1, p2, m1, Symbol(state_secret), Symbol(method_secret)' ); }); }); diff --git a/modules/signals/spec/with-state.spec.ts b/modules/signals/spec/with-state.spec.ts index a86ec195dc..b09ab24cfd 100644 --- a/modules/signals/spec/with-state.spec.ts +++ b/modules/signals/spec/with-state.spec.ts @@ -55,6 +55,8 @@ describe('withState', () => { }); it('logs warning if previously defined signal store members have the same name', () => { + const COMPUTED_SECRET = Symbol('computed_secret'); + const METHOD_SECRET = Symbol('method_secret'); const initialStore = [ withState({ p1: 10, @@ -63,10 +65,12 @@ describe('withState', () => { withComputed(() => ({ s1: signal('s1').asReadonly(), s2: signal({ s: 2 }).asReadonly(), + [COMPUTED_SECRET]: signal(1).asReadonly(), })), withMethods(() => ({ m1() {}, m2() {}, + [METHOD_SECRET]() {}, })), ].reduce((acc, feature) => feature(acc), getInitialInnerStore()); vi.spyOn(console, 'warn').mockImplementation(); @@ -78,12 +82,14 @@ describe('withState', () => { m: { s: 10 }, m2: { m: 2 }, p3: 'p3', + [COMPUTED_SECRET]: 10, + [METHOD_SECRET]: 100, }))(initialStore); expect(console.warn).toHaveBeenCalledWith( '@ngrx/signals: SignalStore members cannot be overridden.', 'Trying to override:', - 'p2, s2, m2' + 'p2, s2, m2, Symbol(computed_secret), Symbol(method_secret)' ); }); }); diff --git a/modules/signals/src/deep-freeze.ts b/modules/signals/src/deep-freeze.ts index be2e5773ee..aeeb9b1f87 100644 --- a/modules/signals/src/deep-freeze.ts +++ b/modules/signals/src/deep-freeze.ts @@ -1,13 +1,12 @@ declare const ngDevMode: boolean; -export function deepFreeze(target: T): T { +export function deepFreeze(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('ɵ')) { + Reflect.ownKeys(target).forEach((prop) => { + if (String(prop).startsWith('ɵ')) { return; } @@ -31,14 +30,14 @@ export function deepFreeze(target: T): T { return target; } -export function freezeInDevMode(target: T): T { +export function freezeInDevMode(target: T): T { return ngDevMode ? deepFreeze(target) : target; } function hasOwnProperty( target: unknown, - propertyName: string -): target is { [propertyName: string]: unknown } { + propertyName: string | symbol +): target is { [propertyName: string | symbol]: unknown } { return isObjectLike(target) ? Object.prototype.hasOwnProperty.call(target, propertyName) : false; diff --git a/modules/signals/src/signal-store-assertions.ts b/modules/signals/src/signal-store-assertions.ts index 865cddbef2..102b73d4f1 100644 --- a/modules/signals/src/signal-store-assertions.ts +++ b/modules/signals/src/signal-store-assertions.ts @@ -4,7 +4,7 @@ declare const ngDevMode: unknown; export function assertUniqueStoreMembers( store: InnerSignalStore, - newMemberKeys: string[] + newMemberKeys: Array ): void { if (!ngDevMode) { return; @@ -15,7 +15,7 @@ export function assertUniqueStoreMembers( ...store.props, ...store.methods, }; - const overriddenKeys = Object.keys(storeMembers).filter((memberKey) => + const overriddenKeys = Reflect.ownKeys(storeMembers).filter((memberKey) => newMemberKeys.includes(memberKey) ); @@ -23,7 +23,7 @@ export function assertUniqueStoreMembers( console.warn( '@ngrx/signals: SignalStore members cannot be overridden.', 'Trying to override:', - overriddenKeys.join(', ') + overriddenKeys.map((key) => String(key)).join(', ') ); } } diff --git a/modules/signals/src/signal-store.ts b/modules/signals/src/signal-store.ts index 60116a79da..ce70ab8e46 100644 --- a/modules/signals/src/signal-store.ts +++ b/modules/signals/src/signal-store.ts @@ -1355,11 +1355,15 @@ export function signalStore( getInitialInnerStore() ); const { stateSignals, props, methods, hooks } = innerStore; - const storeMembers = { ...stateSignals, ...props, ...methods }; + const storeMembers: Record = { + ...stateSignals, + ...props, + ...methods, + }; (this as any)[STATE_SOURCE] = innerStore[STATE_SOURCE]; - for (const key in storeMembers) { + for (const key of Reflect.ownKeys(storeMembers)) { (this as any)[key] = storeMembers[key]; } diff --git a/modules/signals/src/with-methods.ts b/modules/signals/src/with-methods.ts index cd5252ab0d..092012aed7 100644 --- a/modules/signals/src/with-methods.ts +++ b/modules/signals/src/with-methods.ts @@ -30,7 +30,7 @@ export function withMethods< ...store.props, ...store.methods, }); - assertUniqueStoreMembers(store, Object.keys(methods)); + assertUniqueStoreMembers(store, Reflect.ownKeys(methods)); return { ...store, diff --git a/modules/signals/src/with-props.ts b/modules/signals/src/with-props.ts index 6ed348c9b4..5b1dee7e0f 100644 --- a/modules/signals/src/with-props.ts +++ b/modules/signals/src/with-props.ts @@ -28,7 +28,7 @@ export function withProps< ...store.props, ...store.methods, }); - assertUniqueStoreMembers(store, Object.keys(props)); + assertUniqueStoreMembers(store, Reflect.ownKeys(props)); return { ...store, diff --git a/modules/signals/src/with-state.ts b/modules/signals/src/with-state.ts index a6fe37c274..309cfe7a9d 100644 --- a/modules/signals/src/with-state.ts +++ b/modules/signals/src/with-state.ts @@ -32,7 +32,7 @@ export function withState( return (store) => { const state = typeof stateOrFactory === 'function' ? stateOrFactory() : stateOrFactory; - const stateKeys = Object.keys(state); + const stateKeys = Reflect.ownKeys(state); assertUniqueStoreMembers(store, stateKeys); @@ -45,7 +45,7 @@ export function withState( const stateSignals = stateKeys.reduce((acc, key) => { const sliceSignal = computed( - () => (store[STATE_SOURCE]() as Record)[key] + () => (store[STATE_SOURCE]() as Record)[key] ); return { ...acc, [key]: toDeepSignal(sliceSignal) }; }, {} as SignalsDictionary);