fix(signals): assert unique members in withProps feature; add more tests

modules/signals/spec/deep-freeze.spec.ts

@@ -1,11 +1,13 @@
-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 DIRECT_SECRET = Symbol('direct secret');
   const SECRET = Symbol('secret');
 
   const initialState = {
@@ -16,7 +18,6 @@ describe('deepFreeze', () => {
     foo: 'bar',
     numbers: [1, 2, 3],
     ngrx: 'signals',
-    [DIRECT_SECRET]: 'secret',
     nestedSymbol: {
       [SECRET]: 'another secret',
     },
@@ -63,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();
@@ -94,28 +96,26 @@ describe('deepFreeze', () => {
           );
         });
 
-        describe('symbol', () => {
-          it('throws on a mutable change on property of a frozen symbol', () => {
-            const state = stateFactory();
-            const s = getState(state);
+        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"
-            );
-          });
+          expect(() => {
+            s[SECRET].code = 'mutable change';
+          }).toThrowError(
+            "Cannot assign to read only property 'code' of object"
+          );
+        });
 
-          it('throws on a mutable change on nested symbol', () => {
-            const state = stateFactory();
-            const s = getState(state);
+        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"
-            );
-          });
+          expect(() => {
+            s.nestedSymbol[SECRET] = 'mutable change';
+          }).toThrowError(
+            "Cannot assign to read only property 'Symbol(secret)' of object"
+          );
         });
       });
     });

modules/signals/spec/signal-store.spec.ts

@@ -152,7 +152,7 @@ describe('signalStore', () => {
       expect(store.foo()).toBe('foo');
     });
 
-    it('can have symbols as keys as well', () => {
+    it('allows symbols as state keys', () => {
       const SECRET = Symbol('SECRET');
       const Store = signalStore(withState({ [SECRET]: 'bar' }));
       const store = new Store();
@@ -198,7 +198,7 @@ describe('signalStore', () => {
       expect(store.foo).toBe('bar');
     });
 
-    it('allows symbols as props', () => {
+    it('allows symbols as property keys', () => {
       const SECRET = Symbol('SECRET');
 
       const Store = signalStore(withProps(() => ({ [SECRET]: 'secret' })));
@@ -209,38 +209,14 @@ describe('signalStore', () => {
       expect(store[SECRET]).toBe('secret');
     });
 
-    it('allows numbers as props', () => {
+    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');
     });
-
-    it('passes on a symbol to the features', () => {
-      const SECRET = Symbol('SECRET');
-      const SecretStore = signalStore(
-        { providedIn: 'root' },
-        withProps(() => ({
-          [SECRET]: 'not your business',
-        })),
-        withMethods((store) => ({
-          reveil() {
-            return store[SECRET];
-          },
-        })),
-        withComputed((state) => ({
-          secret: computed(() => state[SECRET]),
-        }))
-      );
-
-      const secretStore = TestBed.inject(SecretStore);
-
-      expect(secretStore.reveil()).toBe('not your business');
-      expect(secretStore.secret()).toBe('not your business');
-      expect(secretStore[SECRET]).toBe('not your business');
-    });
   });
 
   describe('withComputed', () => {
@@ -280,24 +256,18 @@ describe('signalStore', () => {
       expect(store.bar()).toBe('bar');
     });
 
-    it('can also expose a symbol', () => {
+    it('allows symbols as computed keys', () => {
       const SECRET = Symbol('SECRET');
       const SecretStore = signalStore(
         { providedIn: 'root' },
         withComputed(() => ({
           [SECRET]: computed(() => 'secret'),
-        })),
-        withMethods((store) => ({
-          reveil() {
-            return store[SECRET];
-          },
         }))
       );
 
       const secretStore = TestBed.inject(SecretStore);
-      const secretSignal = secretStore.reveil();
 
-      expect(secretSignal()).toBe('secret');
+      expect(secretStore[SECRET]()).toBe('secret');
     });
   });
 
@@ -342,7 +312,7 @@ describe('signalStore', () => {
       expect(store.baz()).toBe('baz');
     });
 
-    it('can also expose a symbol', () => {
+    it('allows symbols as method keys', () => {
       const SECRET = Symbol('SECRET');
       const SecretStore = signalStore(
         { providedIn: 'root' },
@@ -546,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');
+    });
   });
 });

modules/signals/spec/state-source.spec.ts

@@ -81,7 +81,7 @@ describe('StateSource', () => {
           });
         });
 
-        it('patches property with symbol keys', () => {
+        it('patches state slice with symbol key', () => {
           const state = stateFactory();
 
           patchState(state, { [SECRET]: 'another secret' });

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)'
     );
   });
 });

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)'
     );
   });
 });

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)'
     );
   });
 });

modules/signals/src/signal-store-assertions.ts

@@ -4,7 +4,7 @@ declare const ngDevMode: unknown;
 
 export function assertUniqueStoreMembers(
   store: InnerSignalStore,
-  newMemberKeys: (string | symbol)[]
+  newMemberKeys: Array<string | symbol>
 ): void {
   if (!ngDevMode) {
     return;

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,