Skip to content

Commit 306ed5a

Browse files
feat(signals): disallow user-defined signals in withState and signalState (#4879)
1 parent bae9f18 commit 306ed5a

File tree

17 files changed

+377
-669
lines changed

17 files changed

+377
-669
lines changed

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

Lines changed: 169 additions & 226 deletions
Large diffs are not rendered by default.

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

Lines changed: 22 additions & 146 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
11
import {
2-
computed,
32
createEnvironmentInjector,
43
effect,
54
EnvironmentInjector,
65
Injectable,
7-
isSignal,
8-
linkedSignal,
9-
resource,
106
signal,
117
} from '@angular/core';
128
import { TestBed } from '@angular/core/testing';
@@ -55,7 +51,7 @@ describe('StateSource', () => {
5551
});
5652

5753
it('returns false for a readonly StateSource', () => {
58-
const stateSource: StateSource<{ vaulue: typeof initialState }> = {
54+
const stateSource: StateSource<{ value: typeof initialState }> = {
5955
[STATE_SOURCE]: { value: signal(initialState).asReadonly() },
6056
};
6157

@@ -212,41 +208,42 @@ describe('StateSource', () => {
212208
});
213209

214210
it('sets only root properties which values have changed (equal check)', () => {
215-
let updateCounter = 0;
216-
const userSignal = signal(
217-
{
218-
firstName: 'John',
219-
lastName: 'Smith',
220-
},
221-
{
222-
equal: (a, b) => {
223-
updateCounter++;
224-
return a === b;
225-
},
226-
}
227-
);
228-
229211
const UserStore = signalStore(
230212
{ providedIn: 'root', protectedState: false },
231-
withState({ user: userSignal, city: 'Changan' })
213+
withState({
214+
user: { firstName: 'John', lastName: 'Smith' },
215+
city: 'Changan',
216+
})
232217
);
233218
const store = TestBed.inject(UserStore);
219+
let userChangedCount = 0;
220+
TestBed.runInInjectionContext(() => {
221+
effect(() => {
222+
store.user();
223+
userChangedCount++;
224+
});
225+
});
234226

235-
expect(updateCounter).toBe(0);
227+
TestBed.tick();
228+
expect(userChangedCount).toBe(1);
236229

237230
patchState(store, { city: 'Xian' });
238-
expect(updateCounter).toBe(0);
231+
TestBed.tick();
232+
expect(userChangedCount).toBe(1);
239233

240234
patchState(store, (state) => state);
241-
expect(updateCounter).toBe(0);
235+
TestBed.tick();
236+
expect(userChangedCount).toBe(1);
242237

243238
patchState(store, ({ user }) => ({ user }));
244-
expect(updateCounter).toBe(0);
239+
TestBed.tick();
240+
expect(userChangedCount).toBe(1);
245241

246242
patchState(store, ({ user }) => ({
247243
user: { ...user, firstName: 'Jane' },
248244
}));
249-
expect(updateCounter).toBe(1);
245+
TestBed.tick();
246+
expect(userChangedCount).toBe(2);
250247
});
251248
});
252249

@@ -468,125 +465,4 @@ describe('StateSource', () => {
468465
});
469466
});
470467
});
471-
472-
describe('user-defined signals as state slices', () => {
473-
[
474-
{
475-
name: 'signalStore',
476-
setup<State extends object>(state: State) {
477-
const Store = signalStore(
478-
{ providedIn: 'root', protectedState: false },
479-
withState(state)
480-
);
481-
return TestBed.inject(Store);
482-
},
483-
},
484-
{
485-
name: 'signalState',
486-
setup<State extends object>(state: State) {
487-
return signalState(state);
488-
},
489-
},
490-
].forEach(({ name, setup }) => {
491-
describe(name, () => {
492-
it('integrates writable Signals as-is', () => {
493-
const user = {
494-
id: 1,
495-
name: 'John Doe',
496-
};
497-
const userSignal = signal(user);
498-
499-
const store = setup({ user: userSignal });
500-
const prettyName = computed(
501-
() => `${store.user().name} ${store.user().id}`
502-
);
503-
504-
expect(store.user()).toBe(user);
505-
expect(prettyName()).toBe('John Doe 1');
506-
507-
userSignal.set({ id: 2, name: 'Jane Doe' });
508-
expect(store.user()).toEqual({ id: 2, name: 'Jane Doe' });
509-
510-
patchState(store, { user: { id: 3, name: 'Jack Doe' } });
511-
expect(store.user()).toEqual({ id: 3, name: 'Jack Doe' });
512-
});
513-
514-
it('integrates a linkedSignal and its update mechanism', () => {
515-
const triggerSignal = signal(1);
516-
const userLinkedSignal = linkedSignal({
517-
source: triggerSignal,
518-
computation: () => ({ id: 1, name: 'John Doe' }),
519-
});
520-
521-
const store = setup({ user: userLinkedSignal });
522-
const prettyName = computed(
523-
() => `${store.user().name} ${store.user().id}`
524-
);
525-
526-
expect(store.user()).toEqual({ id: 1, name: 'John Doe' });
527-
expect(prettyName()).toBe('John Doe 1');
528-
529-
patchState(store, { user: { id: 2, name: 'Jane Doe' } });
530-
expect(prettyName()).toBe('Jane Doe 2');
531-
532-
triggerSignal.set(2);
533-
expect(prettyName()).toBe('John Doe 1');
534-
});
535-
536-
it('supports a resource', async () => {
537-
const resourceTrigger = signal(1);
538-
const userResource = TestBed.runInInjectionContext(() =>
539-
resource({
540-
request: resourceTrigger,
541-
loader: (params) =>
542-
Promise.resolve({ id: params.request, name: 'John Doe' }),
543-
defaultValue: { id: 0, name: 'Loading...' },
544-
})
545-
);
546-
547-
const store = setup({ user: userResource.value });
548-
expect(store.user()).toEqual({ id: 0, name: 'Loading...' });
549-
550-
await new Promise((resolve) => setTimeout(resolve));
551-
expect(store.user()).toEqual({ id: 1, name: 'John Doe' });
552-
553-
resourceTrigger.set(2);
554-
await new Promise((resolve) => setTimeout(resolve));
555-
expect(store.user()).toEqual({ id: 2, name: 'John Doe' });
556-
});
557-
558-
it('allows mixing signals and plain values', () => {
559-
const user = {
560-
id: 1,
561-
name: 'John Doe',
562-
};
563-
const userSignal = signal(user);
564-
const product = { id: 1, name: 'Product A' };
565-
566-
const store = setup({ user: userSignal, product });
567-
568-
expect(store.user()).toBe(user);
569-
expect(store.product()).toBe(product);
570-
});
571-
572-
it('does not strip a readonly Signal', () => {
573-
const store = setup({ n: signal(1).asReadonly() });
574-
575-
expect(isSignal(store.n())).toBe(true);
576-
expect(store.n()()).toBe(1);
577-
});
578-
579-
it('does not strip a nested writable Signal', () => {
580-
const user = {
581-
id: 1,
582-
name: 'John Doe',
583-
};
584-
const userSignal = signal(user);
585-
const store = setup({ data: { user: userSignal } });
586-
587-
expect(isSignal(store.data.user())).toBe(true);
588-
});
589-
});
590-
});
591-
});
592468
});

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,15 +145,15 @@ describe('signalState', () => {
145145
'unique symbol | unique symbol'
146146
);
147147

148-
expectSnippet(snippet).toInfer('setStateValue', 'StateResult<Set<number>>');
148+
expectSnippet(snippet).toInfer('setStateValue', 'Set<number>');
149149
expectSnippet(snippet).toInfer(
150150
'setStateKeys',
151151
'unique symbol | unique symbol'
152152
);
153153

154154
expectSnippet(snippet).toInfer(
155155
'mapStateValue',
156-
'StateResult<Map<number, { bar: boolean; }>>'
156+
'Map<number, { bar: boolean; }>'
157157
);
158158
expectSnippet(snippet).toInfer(
159159
'mapStateKeys',
@@ -162,7 +162,7 @@ describe('signalState', () => {
162162

163163
expectSnippet(snippet).toInfer(
164164
'uintArrayStateValue',
165-
'StateResult<Uint8ClampedArray<ArrayBuffer>>'
165+
'Uint8ClampedArray<ArrayBuffer>'
166166
);
167167
expectSnippet(snippet).toInfer(
168168
'uintArrayStateKeys',
@@ -193,26 +193,26 @@ describe('signalState', () => {
193193

194194
expectSnippet(snippet).toInfer(
195195
'weakSetStateValue',
196-
'StateResult<WeakSet<{ foo: string; }>>'
196+
'WeakSet<{ foo: string; }>'
197197
);
198198
expectSnippet(snippet).toInfer(
199199
'weakSetStateKeys',
200200
'unique symbol | unique symbol'
201201
);
202202

203-
expectSnippet(snippet).toInfer('dateStateValue', 'StateResult<Date>');
203+
expectSnippet(snippet).toInfer('dateStateValue', 'Date');
204204
expectSnippet(snippet).toInfer(
205205
'dateStateKeys',
206206
'unique symbol | unique symbol'
207207
);
208208

209-
expectSnippet(snippet).toInfer('errorStateValue', 'StateResult<Error>');
209+
expectSnippet(snippet).toInfer('errorStateValue', 'Error');
210210
expectSnippet(snippet).toInfer(
211211
'errorStateKeys',
212212
'unique symbol | unique symbol'
213213
);
214214

215-
expectSnippet(snippet).toInfer('regExpStateValue', 'StateResult<RegExp>');
215+
expectSnippet(snippet).toInfer('regExpStateValue', 'RegExp');
216216
expectSnippet(snippet).toInfer(
217217
'regExpStateKeys',
218218
'unique symbol | unique symbol'
@@ -228,7 +228,7 @@ describe('signalState', () => {
228228

229229
expectSnippet(snippet).toSucceed();
230230

231-
expectSnippet(snippet).toInfer('stateValue', 'StateResult<() => void>');
231+
expectSnippet(snippet).toInfer('stateValue', '() => void');
232232
expectSnippet(snippet).toInfer(
233233
'stateKeys',
234234
'unique symbol | unique symbol'

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

Lines changed: 1 addition & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { compilerOptions } from './helpers';
44
describe('signalStore', () => {
55
const expectSnippet = expecter(
66
(code) => `
7-
import { computed, inject, Signal, signal } from '@angular/core';
7+
import { computed, inject, Signal } from '@angular/core';
88
import {
99
getState,
1010
patchState,
@@ -860,47 +860,6 @@ describe('signalStore', () => {
860860
`).toFail(/'_count2' does not exist in type/);
861861
});
862862

863-
it('exposes a writable Signal as readonly', () => {
864-
const snippet = `
865-
type User = {
866-
id: number;
867-
name: string;
868-
location: {
869-
city: string;
870-
country: string;
871-
}
872-
}
873-
874-
const userSignal = signal({
875-
id: 1,
876-
name: 'John Doe',
877-
location: {
878-
city: 'New York',
879-
country: 'USA'
880-
}
881-
});
882-
883-
const Store = signalStore(
884-
withState({
885-
user: userSignal,
886-
foo: signal('bar')
887-
})
888-
);
889-
890-
const store = new Store();
891-
const user = store.user;
892-
const foo = store.foo;
893-
`;
894-
895-
expectSnippet(snippet).toSucceed();
896-
897-
expectSnippet(snippet).toInfer(
898-
'user',
899-
'DeepSignal<{ id: number; name: string; location: { city: string; country: string; }; }>'
900-
);
901-
expectSnippet(snippet).toInfer('foo', 'Signal<string>');
902-
});
903-
904863
describe('custom features', () => {
905864
const baseSnippet = `
906865
function withX() {

modules/signals/spec/types/with-linked-state.types.spec.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ describe('withLinkedState', () => {
5959
);
6060
});
6161

62-
it('resolves to a normal state signal with automatic linkedSignal', () => {
62+
it('adds state slice with computation function', () => {
6363
const snippet = `
6464
const UserStore = signalStore(
6565
{ providedIn: 'root' },
@@ -78,7 +78,7 @@ describe('withLinkedState', () => {
7878
expectSnippet(snippet).toInfer('lastname', 'Signal<string>');
7979
});
8080

81-
it('resolves to a normal state signal with manual linkedSignal', () => {
81+
it('adds state slice with explicit linkedSignal', () => {
8282
const snippet = `
8383
const UserStore = signalStore(
8484
{ providedIn: 'root' },
@@ -100,7 +100,7 @@ describe('withLinkedState', () => {
100100
expectSnippet(snippet).toInfer('lastname', 'Signal<string>');
101101
});
102102

103-
it('sets stateSignals as DeepSignal for automatic linkedSignal', () => {
103+
it('creates deep signals with computation functions', () => {
104104
const snippet = `
105105
const UserStore = signalStore(
106106
{ providedIn: 'root' },
@@ -128,7 +128,7 @@ describe('withLinkedState', () => {
128128
);
129129
});
130130

131-
it('sets stateSignals as DeepSignal for manual linkedSignal', () => {
131+
it('creates deep signals with explicit linked signals', () => {
132132
const snippet = `
133133
const UserStore = signalStore(
134134
{ providedIn: 'root' },
@@ -163,18 +163,21 @@ describe('withLinkedState', () => {
163163
withLinkedState(({ foo }) => ({
164164
bar: () => foo(),
165165
baz: linkedSignal(() => foo()),
166+
qux: signal({ x: 1 }),
166167
}))
167168
);
168169
169170
const store = new Store();
170171
171172
const bar = store.bar;
172173
const baz = store.baz;
174+
const qux = store.qux;
173175
`;
174176

175177
expectSnippet(snippet).toSucceed();
176178

177179
expectSnippet(snippet).toInfer('bar', 'Signal<string>');
178180
expectSnippet(snippet).toInfer('baz', 'Signal<string>');
181+
expectSnippet(snippet).toInfer('qux', 'DeepSignal<{ x: number; }>');
179182
});
180183
});

0 commit comments

Comments
 (0)