Skip to content

Commit

Permalink
fix(signals): enable withProps to handle Symbols (#4656)
Browse files Browse the repository at this point in the history
Closes #4655
  • Loading branch information
rainerhahnekamp authored Jan 24, 2025
1 parent 1f5ec46 commit 02320b3
Show file tree
Hide file tree
Showing 13 changed files with 179 additions and 26 deletions.
45 changes: 40 additions & 5 deletions modules/signals/spec/deep-freeze.spec.ts
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -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 [
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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"
);
});
});
});
}
Expand Down
86 changes: 85 additions & 1 deletion modules/signals/spec/signal-store.spec.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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');
});
});
});
10 changes: 10 additions & 0 deletions modules/signals/spec/state-source.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -27,6 +29,7 @@ describe('StateSource', () => {
foo: 'bar',
numbers: [1, 2, 3],
ngrx: 'signals',
[SECRET]: 'secret',
};

describe('patchState', () => {
Expand Down Expand Up @@ -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();

Expand Down
5 changes: 4 additions & 1 deletion modules/signals/spec/with-computed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -27,6 +28,7 @@ describe('withComputed', () => {
withComputed(() => ({
s1: signal('s1').asReadonly(),
s2: signal({ s: 2 }).asReadonly(),
[COMPUTED_SECRET]: signal(1).asReadonly(),
})),
withMethods(() => ({
m1() {},
Expand All @@ -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)'
);
});
});
8 changes: 7 additions & 1 deletion modules/signals/spec/with-methods.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {},
Expand All @@ -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)'
);
});
});
8 changes: 7 additions & 1 deletion modules/signals/spec/with-props.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -29,6 +32,7 @@ describe('withProps', () => {
withMethods(() => ({
m1() {},
m2() {},
[METHOD_SECRET]() {},
})),
].reduce((acc, feature) => feature(acc), getInitialInnerStore());
vi.spyOn(console, 'warn').mockImplementation();
Expand All @@ -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)'
);
});
});
8 changes: 7 additions & 1 deletion modules/signals/spec/with-state.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand All @@ -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)'
);
});
});
Loading

0 comments on commit 02320b3

Please sign in to comment.