fix(signals): enable withProps to handle Symbols (#4656)

Closes #4655

Author: rainerhahnekamp

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"
+          );
+        });
       });
     });
   }

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

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();
 

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

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