Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading