Skip to content

Commit 52b0cba

Browse files
refactor!: split up state signal to multiple signals
This is a necessary refactoring to support a state which allows different types of Signals, i.e. `linkedSignal` & `signal` but still presents itself as a single unit to the outside. This commit splits the single Signal of `STATE_SOURCE`, which contains the state in the `SignalStore` and `signalState`, into multiple Signals. An example. Given the following type: ```typescript type User = { firstname: string; lastname: string; }; ``` Before, `STATE_SOURCE` would have been the following type: ```typescript WritableSignal<User>; ``` With this change, it is: ```typescript { firstname: WritableSignal<string>; lastname: WritableSignal<string>; } ``` Most changes affect the tests which focus on the `STATE_SOURCE`. Except for one test in `signal-state.spec.ts` ('does not modify STATE_SOURCE'), all tests could be updated to assert the new behavior. ## Breaking Changes - Any code which accesses the hidden `STATE_SOURCE` will be impacted. - Breaking Changes to the public behavior are rare. ### different object reference for state of `STATE_SOURCE` `STATE_SOURCE` does not keep the object reference of the initial state upon initialization. ```typescript const initialObject = { ngrx: 'rocks', }; const state = signalState(initialObject); // before state() === initialObject; // ✅ // after state() === initialObject; // ❌ ``` ### no Signal change on empty patched state `patchState` created a clone of the state and applied the patches to the clone. That meant, if nothing was changed, the Signal still fired because of the shallow clone. Since the state Signal doesn't exist anymore, there will be no change in this case. ```typescript const state = signalState({ ngrx: 'rocks' }); let changeCount = 0; effect(() => { changeCount++; }); TestBed.flushEffects(); expect(changeCount).toBe(1); patchState(state, {}); // before expect(changeCount).toBe(2); // ✅ // after expect(changeCount).toBe(2); // ❌ changeCount is still 1 ``` ## Further Changes - `signalStoreFeature` had to be refactored because of typing issues with the change to `WritableStateSource`. - `patchState` get the `NoInfer` for `updaters` argument. Otherwise, with multiple updaters, the former updater would have defined the `State` for the next updater.
1 parent f966d0a commit 52b0cba

14 files changed

+231
-116
lines changed

modules/signals/spec/helpers.ts

+24
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Component, inject, Type } from '@angular/core';
22
import { TestBed } from '@angular/core/testing';
3+
import { SignalsDictionary } from '../src/signal-store-models';
34

45
export function createLocalService<Service extends Type<unknown>>(
56
serviceToken: Service
@@ -30,3 +31,26 @@ export function createLocalService<Service extends Type<unknown>>(
3031
destroy: () => fixture.destroy(),
3132
};
3233
}
34+
35+
/**
36+
* This could be done by using `getState`, but
37+
* 1. We don't want to depend on the implementation of `getState` in the test.
38+
* 2. We want to be able to provide the state in its actual type (with slice signals).
39+
*/
40+
export function assertStateSource(
41+
state: SignalsDictionary,
42+
expected: SignalsDictionary
43+
): void {
44+
const stateKeys = Reflect.ownKeys(state);
45+
const expectedKeys = Reflect.ownKeys(expected);
46+
47+
const currentState = stateKeys.reduce((acc, key) => {
48+
acc[key] = state[key]();
49+
return acc;
50+
}, {} as Record<string | symbol, unknown>);
51+
const expectedState = expectedKeys.reduce((acc, key) => {
52+
acc[key] = expected[key]();
53+
return acc;
54+
}, {} as Record<string | symbol, unknown>);
55+
expect(currentState).toEqual(expectedState);
56+
}

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

+21-21
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { computed } from '@angular/core';
2-
import { effect, isSignal } from '@angular/core';
1+
import { computed, effect, isSignal } from '@angular/core';
32
import { TestBed } from '@angular/core/testing';
43
import { patchState, signalState } from '../src';
4+
import { SignalsDictionary } from '../src/signal-store-models';
55
import { STATE_SOURCE } from '../src/state-source';
66

77
vi.mock('@angular/core', { spy: true });
@@ -21,21 +21,30 @@ describe('signalState', () => {
2121
vi.clearAllMocks();
2222
});
2323

24-
it('has writable state source', () => {
25-
const state = signalState({});
26-
const stateSource = state[STATE_SOURCE];
24+
it('creates its properites as Signals', () => {
25+
const state = signalState({ foo: 'bar' });
26+
const stateSource: SignalsDictionary = state[STATE_SOURCE];
2727

28-
expect(isSignal(stateSource)).toBe(true);
29-
expect(typeof stateSource.update === 'function').toBe(true);
28+
expect(isSignal(state)).toBe(true);
29+
for (const key of Reflect.ownKeys(stateSource)) {
30+
expect(isSignal(stateSource[key])).toBe(true);
31+
expect(typeof stateSource[key].update === 'function').toBe(true);
32+
}
33+
});
34+
35+
it('does not keep the object reference of the initial state', () => {
36+
const state = signalState(initialState);
37+
expect(state()).not.toBe(initialState);
38+
expect(state()).toEqual(initialState);
3039
});
3140

3241
it('creates signals for nested state slices', () => {
3342
const state = signalState(initialState);
3443

35-
expect(state()).toBe(initialState);
44+
expect(state()).toEqual(initialState);
3645
expect(isSignal(state)).toBe(true);
3746

38-
expect(state.user()).toBe(initialState.user);
47+
expect(state.user()).toEqual(initialState.user);
3948
expect(isSignal(state.user)).toBe(true);
4049

4150
expect(state.user.firstName()).toBe(initialState.user.firstName);
@@ -80,20 +89,11 @@ describe('signalState', () => {
8089
expect((state.user.firstName as any).y).toBe(undefined);
8190
});
8291

83-
it('does not modify STATE_SOURCE', () => {
84-
const state = signalState(initialState);
85-
86-
expect((state[STATE_SOURCE] as any).user).toBe(undefined);
87-
expect((state[STATE_SOURCE] as any).foo).toBe(undefined);
88-
expect((state[STATE_SOURCE] as any).numbers).toBe(undefined);
89-
expect((state[STATE_SOURCE] as any).ngrx).toBe(undefined);
90-
});
91-
9292
it('overrides Function properties if state keys have the same name', () => {
9393
const initialState = { name: { length: { length: 'ngrx' }, name: 20 } };
9494
const state = signalState(initialState);
9595

96-
expect(state()).toBe(initialState);
96+
expect(state()).toEqual(initialState);
9797

9898
expect(state.name()).toBe(initialState.name);
9999
expect(isSignal(state.name)).toBe(true);
@@ -190,12 +190,12 @@ describe('signalState', () => {
190190

191191
patchState(state, {});
192192
TestBed.flushEffects();
193-
expect(stateCounter).toBe(2);
193+
expect(stateCounter).toBe(1);
194194
expect(userCounter).toBe(1);
195195

196196
patchState(state, (state) => state);
197197
TestBed.flushEffects();
198-
expect(stateCounter).toBe(3);
198+
expect(stateCounter).toBe(1);
199199
expect(userCounter).toBe(1);
200200
}));
201201
});

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

+8-3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
withState,
99
} from '../src';
1010
import { STATE_SOURCE } from '../src/state-source';
11+
import { assertStateSource } from './helpers';
1112

1213
describe('signalStoreFeature', () => {
1314
function withCustomFeature1() {
@@ -50,7 +51,7 @@ describe('signalStoreFeature', () => {
5051

5152
const store = new Store();
5253

53-
expect(store[STATE_SOURCE]()).toEqual({ foo: 'foo' });
54+
assertStateSource(store[STATE_SOURCE], { foo: signal('foo') });
5455
expect(store.foo()).toBe('foo');
5556
expect(store.bar()).toBe('foo1');
5657
expect(store.baz()).toBe('foofoo12');
@@ -65,7 +66,7 @@ describe('signalStoreFeature', () => {
6566

6667
const store = new Store();
6768

68-
expect(store[STATE_SOURCE]()).toEqual({ foo: 'foo' });
69+
assertStateSource(store[STATE_SOURCE], { foo: signal('foo') });
6970
expect(store.foo()).toBe('foo');
7071
expect(store.bar()).toBe('foo1');
7172
expect(store.m()).toBe('foofoofoo123');
@@ -81,7 +82,11 @@ describe('signalStoreFeature', () => {
8182

8283
const store = new Store();
8384

84-
expect(store[STATE_SOURCE]()).toEqual({ foo: 'foo', foo1: 1, foo2: 2 });
85+
assertStateSource(store[STATE_SOURCE], {
86+
foo: signal('foo'),
87+
foo1: signal(1),
88+
foo2: signal(2),
89+
});
8590
expect(store.foo()).toBe('foo');
8691
expect(store.bar()).toBe('foo1');
8792
expect(store.baz()).toBe('foofoo12');

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

+45-20
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
withState,
1717
} from '../src';
1818
import { STATE_SOURCE } from '../src/state-source';
19-
import { createLocalService } from './helpers';
19+
import { assertStateSource, createLocalService } from './helpers';
2020

2121
describe('signalStore', () => {
2222
describe('creation', () => {
@@ -47,42 +47,56 @@ describe('signalStore', () => {
4747
expect(store1.foo()).toBe('bar');
4848
});
4949

50-
it('creates a store with readonly state source by default', () => {
50+
it('creates a store with state source as Record holding slices as signals by default', () => {
5151
const Store = signalStore(withState({ foo: 'bar' }));
5252
const store = new Store();
5353
const stateSource = store[STATE_SOURCE];
5454

55-
expect(isSignal(stateSource)).toBe(true);
56-
expect(stateSource()).toEqual({ foo: 'bar' });
55+
expect(isSignal(stateSource)).toBe(false);
56+
expect(Object.keys(stateSource)).toEqual(['foo']);
57+
expect(isSignal(stateSource.foo)).toBe(true);
58+
assertStateSource(stateSource, {
59+
foo: signal('bar'),
60+
});
5761
});
5862

59-
it('creates a store with readonly state source when protectedState option is true', () => {
63+
it('creates a store with state source as Record holding slices as signals when protectedState option is true', () => {
6064
const Store = signalStore(
6165
{ protectedState: true },
6266
withState({ foo: 'bar' })
6367
);
6468
const store = new Store();
6569
const stateSource = store[STATE_SOURCE];
6670

67-
expect(isSignal(stateSource)).toBe(true);
68-
expect(stateSource()).toEqual({ foo: 'bar' });
71+
expect(isSignal(stateSource)).toBe(false);
72+
expect(Object.keys(stateSource)).toEqual(['foo']);
73+
expect(isSignal(stateSource.foo)).toBe(true);
74+
assertStateSource(stateSource, {
75+
foo: signal('bar'),
76+
});
6977
});
7078

71-
it('creates a store with writable state source when protectedState option is false', () => {
79+
it('creates a store with state source as Record holding slices as writeable signals when protectedState option is false', () => {
7280
const Store = signalStore(
7381
{ protectedState: false },
7482
withState({ foo: 'bar' })
7583
);
7684
const store = new Store();
7785
const stateSource = store[STATE_SOURCE];
7886

79-
expect(isSignal(stateSource)).toBe(true);
80-
expect(stateSource()).toEqual({ foo: 'bar' });
81-
expect(typeof stateSource.update === 'function').toBe(true);
87+
expect(isSignal(stateSource)).toBe(false);
88+
expect(Object.keys(stateSource)).toEqual(['foo']);
89+
expect(isSignal(stateSource.foo)).toBe(true);
90+
assertStateSource(stateSource, {
91+
foo: signal('bar'),
92+
});
93+
expect(typeof stateSource.foo.update === 'function').toBe(true);
8294

8395
patchState(store, { foo: 'baz' });
8496

85-
expect(stateSource()).toEqual({ foo: 'baz' });
97+
assertStateSource(stateSource, {
98+
foo: signal('baz'),
99+
});
86100
});
87101
});
88102

@@ -97,10 +111,11 @@ describe('signalStore', () => {
97111

98112
const store = new Store();
99113

100-
expect(store[STATE_SOURCE]()).toEqual({
101-
foo: 'foo',
102-
x: { y: { z: 10 } },
114+
assertStateSource(store[STATE_SOURCE], {
115+
foo: signal('foo'),
116+
x: signal({ y: { z: 10 } }),
103117
});
118+
104119
expect(store.foo()).toBe('foo');
105120
expect(store.x()).toEqual({ y: { z: 10 } });
106121
expect(store.x.y()).toEqual({ z: 10 });
@@ -178,7 +193,9 @@ describe('signalStore', () => {
178193

179194
const store = new Store();
180195

181-
expect(store[STATE_SOURCE]()).toEqual({ foo: 'foo' });
196+
assertStateSource(store[STATE_SOURCE], {
197+
foo: signal('foo'),
198+
});
182199
expect(store.foo()).toBe('foo');
183200
expect(store.bar()).toBe('bar');
184201
expect(store.num).toBe(10);
@@ -236,7 +253,9 @@ describe('signalStore', () => {
236253

237254
const store = new Store();
238255

239-
expect(store[STATE_SOURCE]()).toEqual({ foo: 'foo' });
256+
assertStateSource(store[STATE_SOURCE], {
257+
foo: signal('foo'),
258+
});
240259
expect(store.foo()).toBe('foo');
241260
expect(store.bar()).toBe('bar');
242261
expect(store.num).toBe(10);
@@ -279,7 +298,9 @@ describe('signalStore', () => {
279298
withMethods(() => ({ baz: () => 'baz' })),
280299
withProps(() => ({ num: 100 })),
281300
withMethods((store) => {
282-
expect(store[STATE_SOURCE]()).toEqual({ foo: 'foo' });
301+
assertStateSource(store[STATE_SOURCE], {
302+
foo: signal('foo'),
303+
});
283304
expect(store.foo()).toBe('foo');
284305
expect(store.bar()).toBe('bar');
285306
expect(store.baz()).toBe('baz');
@@ -291,7 +312,9 @@ describe('signalStore', () => {
291312

292313
const store = new Store();
293314

294-
expect(store[STATE_SOURCE]()).toEqual({ foo: 'foo' });
315+
assertStateSource(store[STATE_SOURCE], {
316+
foo: signal('foo'),
317+
});
295318
expect(store.foo()).toBe('foo');
296319
expect(store.bar()).toBe('bar');
297320
expect(store.baz()).toBe('baz');
@@ -372,7 +395,9 @@ describe('signalStore', () => {
372395
withProps(() => ({ num: 10 })),
373396
withHooks({
374397
onInit(store) {
375-
expect(store[STATE_SOURCE]()).toEqual({ foo: 'foo' });
398+
assertStateSource(store[STATE_SOURCE], {
399+
foo: signal('foo'),
400+
});
376401
expect(store.foo()).toBe('foo');
377402
expect(store.bar()).toBe('bar');
378403
expect(store.baz()).toBe('baz');

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

+23-19
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,9 @@ import {
1717
withHooks,
1818
withMethods,
1919
withState,
20-
WritableStateSource,
2120
} from '../src';
2221
import { STATE_SOURCE } from '../src/state-source';
23-
import { createLocalService } from './helpers';
22+
import { assertStateSource, createLocalService } from './helpers';
2423

2524
const SECRET = Symbol('SECRET');
2625

@@ -38,16 +37,16 @@ describe('StateSource', () => {
3837

3938
describe('isWritableStateSource', () => {
4039
it('returns true for a writable StateSource', () => {
41-
const stateSource: StateSource<typeof initialState> = {
42-
[STATE_SOURCE]: signal(initialState),
40+
const stateSource: StateSource<{ value: typeof initialState }> = {
41+
[STATE_SOURCE]: { value: signal(initialState) },
4342
};
4443

4544
expect(isWritableStateSource(stateSource)).toBe(true);
4645
});
4746

4847
it('returns false for a readonly StateSource', () => {
49-
const stateSource: StateSource<typeof initialState> = {
50-
[STATE_SOURCE]: signal(initialState).asReadonly(),
48+
const stateSource: StateSource<{ vaulue: typeof initialState }> = {
49+
[STATE_SOURCE]: { value: signal(initialState).asReadonly() },
5150
};
5251

5352
expect(isWritableStateSource(stateSource)).toBe(false);
@@ -81,10 +80,12 @@ describe('StateSource', () => {
8180
foo: 'baz',
8281
});
8382

84-
expect(state[STATE_SOURCE]()).toEqual({
85-
...initialState,
86-
user: { firstName: 'Johannes', lastName: 'Schmidt' },
87-
foo: 'baz',
83+
assertStateSource(state[STATE_SOURCE], {
84+
user: signal({ firstName: 'Johannes', lastName: 'Schmidt' }),
85+
foo: signal('baz'),
86+
numbers: signal([1, 2, 3]),
87+
ngrx: signal('signals'),
88+
[SECRET]: signal('secret'),
8889
});
8990
});
9091

@@ -96,10 +97,12 @@ describe('StateSource', () => {
9697
ngrx: 'rocks',
9798
}));
9899

99-
expect(state[STATE_SOURCE]()).toEqual({
100-
...initialState,
101-
numbers: [1, 2, 3, 4],
102-
ngrx: 'rocks',
100+
assertStateSource(state[STATE_SOURCE], {
101+
user: signal({ firstName: 'John', lastName: 'Smith' }),
102+
foo: signal('bar'),
103+
numbers: signal([1, 2, 3, 4]),
104+
ngrx: signal('rocks'),
105+
[SECRET]: signal('secret'),
103106
});
104107
});
105108

@@ -121,11 +124,12 @@ describe('StateSource', () => {
121124
{ foo: 'foo' }
122125
);
123126

124-
expect(state[STATE_SOURCE]()).toEqual({
125-
...initialState,
126-
user: { firstName: 'Jovan', lastName: 'Schmidt' },
127-
foo: 'foo',
128-
numbers: [1, 2, 3, 4],
127+
assertStateSource(state[STATE_SOURCE], {
128+
user: signal({ firstName: 'Jovan', lastName: 'Schmidt' }),
129+
foo: signal('foo'),
130+
numbers: signal([1, 2, 3, 4]),
131+
ngrx: signal('signals'),
132+
[SECRET]: signal('secret'),
129133
});
130134
});
131135

0 commit comments

Comments
 (0)