Skip to content

Commit ee92912

Browse files
refactor(component-store): fine-tune effect types (#2645)
BREAKING CHANGE: EffectReturnFn has been removed and the effect type is stricter and more predictable. BEFORE: If effect was const e = effect((o: Observable<string>) => ....) it was still possible to call e() without passing any strings AFTER: If effect was const e = effect((o: Observable<string>) => ....) its not allowed to call e() without passing any strings
1 parent 63b8e14 commit ee92912

File tree

4 files changed

+205
-32
lines changed

4 files changed

+205
-32
lines changed

modules/component-store/spec/component-store.spec.ts

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ describe('Component Store', () => {
425425
componentStore.state$.subscribe((state) => results.push(state));
426426

427427
// Update with Observable.
428-
const subsription = updater(
428+
const subscription = updater(
429429
interval(10).pipe(
430430
map((v) => ({ value: String(v) })),
431431
take(10) // just in case
@@ -435,7 +435,7 @@ describe('Component Store', () => {
435435
// Advance for 40 fake milliseconds and unsubscribe - should capture
436436
// from '0' to '3'
437437
advance(40);
438-
subsription.unsubscribe();
438+
subscription.unsubscribe();
439439

440440
// Advance for 20 more fake milliseconds, to check if anything else
441441
// is captured
@@ -468,7 +468,7 @@ describe('Component Store', () => {
468468
componentStore.state$.subscribe((state) => results.push(state));
469469

470470
// Update with Observable.
471-
const subsription = updater(
471+
const subscription = updater(
472472
interval(10).pipe(
473473
map((v) => ({ value: 'a' + v })),
474474
take(10) // just in case
@@ -486,7 +486,7 @@ describe('Component Store', () => {
486486
// Advance for 40 fake milliseconds and unsubscribe - should capture
487487
// from '0' to '3'
488488
advance(40);
489-
subsription.unsubscribe();
489+
subscription.unsubscribe();
490490

491491
// Advance for 30 more fake milliseconds, to make sure that second
492492
// Observable still emits
@@ -1119,7 +1119,7 @@ describe('Component Store', () => {
11191119
origin$.pipe(tap((v) => results.push(typeof v)))
11201120
);
11211121
const effect = componentStore.effect(mockGenerator);
1122-
effect(undefined);
1122+
effect();
11231123
effect();
11241124

11251125
expect(results).toEqual(['undefined', 'undefined']);
@@ -1130,7 +1130,7 @@ describe('Component Store', () => {
11301130
'is run when observable is provided',
11311131
marbles((m) => {
11321132
const mockGenerator = jest.fn((origin$) => origin$);
1133-
const effect = componentStore.effect(mockGenerator);
1133+
const effect = componentStore.effect<string>(mockGenerator);
11341134

11351135
effect(m.cold('-a-b-c|'));
11361136

@@ -1143,7 +1143,7 @@ describe('Component Store', () => {
11431143
'is run with multiple Observables',
11441144
marbles((m) => {
11451145
const mockGenerator = jest.fn((origin$) => origin$);
1146-
const effect = componentStore.effect(mockGenerator);
1146+
const effect = componentStore.effect<string>(mockGenerator);
11471147

11481148
effect(m.cold('-a-b-c|'));
11491149
effect(m.hot(' --d--e----f-'));
@@ -1170,12 +1170,12 @@ describe('Component Store', () => {
11701170
);
11711171

11721172
// Update with Observable.
1173-
const subsription = effect(observable$);
1173+
const subscription = effect(observable$);
11741174

11751175
// Advance for 40 fake milliseconds and unsubscribe - should capture
11761176
// from '0' to '3'
11771177
advance(40);
1178-
subsription.unsubscribe();
1178+
subscription.unsubscribe();
11791179

11801180
// Advance for 20 more fake milliseconds, to check if anything else
11811181
// is captured
@@ -1196,7 +1196,7 @@ describe('Component Store', () => {
11961196
);
11971197

11981198
// Pass the first Observable to the effect.
1199-
const subsription = effect(
1199+
const subscription = effect(
12001200
interval(10).pipe(
12011201
map((v) => ({ value: 'a' + v })),
12021202
take(10) // just in case
@@ -1214,7 +1214,7 @@ describe('Component Store', () => {
12141214
// Advance for 40 fake milliseconds and unsubscribe - should capture
12151215
// from '0' to '3'
12161216
advance(40);
1217-
subsription.unsubscribe();
1217+
subscription.unsubscribe();
12181218

12191219
// Advance for 30 more fake milliseconds, to make sure that second
12201220
// Observable still emits
@@ -1236,7 +1236,7 @@ describe('Component Store', () => {
12361236
);
12371237

12381238
it('completes when componentStore is destroyed', (doneFn: jest.DoneCallback) => {
1239-
componentStore.effect((origin$) =>
1239+
componentStore.effect((origin$: Observable<number>) =>
12401240
origin$.pipe(
12411241
finalize(() => {
12421242
doneFn();
@@ -1249,7 +1249,7 @@ describe('Component Store', () => {
12491249
});
12501250

12511251
it('observable argument completes when componentStore is destroyed', (doneFn: jest.DoneCallback) => {
1252-
componentStore.effect((origin$) => origin$)(
1252+
componentStore.effect((origin$: Observable<number>) => origin$)(
12531253
interval(10).pipe(
12541254
finalize(() => {
12551255
doneFn();
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
import { expecter } from 'ts-snippet';
2+
import { compilerOptions } from './utils';
3+
4+
describe('ComponentStore types', () => {
5+
describe('effect', () => {
6+
const expectSnippet = expecter(
7+
(code) => `
8+
import { ComponentStore } from '@ngrx/component-store';
9+
import { of, EMPTY, Observable } from 'rxjs';
10+
import { concatMap } from 'rxjs/operators';
11+
12+
const number$: Observable<number> = of(5);
13+
const string$: Observable<string> = of('string');
14+
15+
const componentStore = new ComponentStore();
16+
${code}
17+
`,
18+
compilerOptions()
19+
);
20+
21+
describe('infers Subscription', () => {
22+
it('when argument type is specified and a variable with corresponding type is passed', () => {
23+
expectSnippet(
24+
`const eff = componentStore.effect((e: Observable<string>) => number$)('string');`
25+
).toInfer('eff', 'Subscription');
26+
});
27+
28+
it(
29+
'when argument type is specified, returns EMPTY and ' +
30+
'a variable with corresponding type is passed',
31+
() => {
32+
expectSnippet(
33+
`const eff = componentStore.effect((e: Observable<string>) => EMPTY)('string');`
34+
).toInfer('eff', 'Subscription');
35+
}
36+
);
37+
38+
it('when argument type is specified and an Observable with corresponding type is passed', () => {
39+
expectSnippet(
40+
`const eff = componentStore.effect((e: Observable<string>) => EMPTY)(string$);`
41+
).toInfer('eff', 'Subscription');
42+
});
43+
44+
it('when argument type is specified as Observable<unknown> and any type is passed', () => {
45+
expectSnippet(
46+
`const eff = componentStore.effect((e: Observable<unknown>) => EMPTY)(5);`
47+
).toInfer('eff', 'Subscription');
48+
});
49+
50+
it('when generic type is specified and a variable with corresponding type is passed', () => {
51+
expectSnippet(
52+
`const eff = componentStore.effect<string>((e) => number$)('string');`
53+
).toInfer('eff', 'Subscription');
54+
});
55+
56+
it('when generic type is specified as unknown and a variable with any type is passed', () => {
57+
expectSnippet(
58+
`const eff = componentStore.effect<unknown>((e) => number$)('string');`
59+
).toInfer('eff', 'Subscription');
60+
});
61+
62+
it('when generic type is specified as unknown and origin can still be piped', () => {
63+
expectSnippet(
64+
`const eff = componentStore.effect<unknown>((e) => e.pipe(concatMap(() => of())))('string');`
65+
).toInfer('eff', 'Subscription');
66+
});
67+
68+
it('when generic type is specified as unknown and origin can still be piped', () => {
69+
expectSnippet(
70+
`const eff = componentStore.effect<unknown>((e) => e.pipe(concatMap(() => of())))('string');`
71+
).toInfer('eff', 'Subscription');
72+
});
73+
});
74+
75+
describe('infers void', () => {
76+
it('when argument type is specified as Observable<void> and nothing is passed', () => {
77+
expectSnippet(
78+
`const eff = componentStore.effect((e: Observable<void>) => string$)();`
79+
).toInfer('eff', 'void');
80+
});
81+
82+
it('when type is not specified and origin can still be piped', () => {
83+
expectSnippet(
84+
// treated as Observable<void> 👇
85+
`const eff = componentStore.effect((e) => e.pipe(concatMap(() => of())))();`
86+
).toInfer('eff', 'void');
87+
});
88+
89+
it('when generic type is specified as void and origin can still be piped', () => {
90+
expectSnippet(
91+
`const eff = componentStore.effect<void>((e) => e.pipe(concatMap(() => number$)))();`
92+
).toInfer('eff', 'void');
93+
});
94+
});
95+
96+
describe('catches improper usage', () => {
97+
it('when type is specified and argument is not passed', () => {
98+
expectSnippet(
99+
`componentStore.effect((e: Observable<string>) => of())();`
100+
).toFail(/Expected 1 arguments, but got 0/);
101+
});
102+
103+
it('when type is specified and argument of incorrect type is passed', () => {
104+
expectSnippet(
105+
`componentStore.effect((e: Observable<string>) => number$)(5);`
106+
).toFail(
107+
/Argument of type '5' is not assignable to parameter of type 'string \| Observable<string>'./
108+
);
109+
});
110+
111+
it('when type is specified and Observable argument of incorrect type is passed', () => {
112+
expectSnippet(
113+
`componentStore.effect((e: Observable<string>) => string$)(number$);`
114+
).toFail(
115+
/Argument of type 'Observable<number>' is not assignable to parameter of type 'string \| Observable<string>'/
116+
);
117+
});
118+
119+
it('when argument type is specified as Observable<unknown> and type is not passed', () => {
120+
expectSnippet(
121+
`componentStore.effect((e: Observable<unknown>) => EMPTY)();`
122+
).toFail(/Expected 1 arguments, but got 0/);
123+
});
124+
125+
it('when generic type is specified and a variable with incorrect type is passed', () => {
126+
expectSnippet(
127+
`componentStore.effect<string>((e) => number$)(5);`
128+
).toFail(
129+
/Argument of type '5' is not assignable to parameter of type 'string \| Observable<string>'/
130+
);
131+
});
132+
133+
it('when generic type is specified as unknown and a variable is not passed', () => {
134+
expectSnippet(
135+
`componentStore.effect<unknown>((e) => number$)();`
136+
).toFail(/Expected 1 arguments, but got 0/);
137+
});
138+
139+
it('when argument type is specified as Observable<void> and anything is passed', () => {
140+
expectSnippet(
141+
`componentStore.effect((e: Observable<void>) => string$)(5);`
142+
).toFail(/Expected 0 arguments, but got 1/);
143+
});
144+
145+
it('when type is not specified and anything is passed', () => {
146+
expectSnippet(
147+
`const eff = componentStore.effect((e) => EMPTY)('string');`
148+
).toFail(/Expected 0 arguments, but got 1/);
149+
});
150+
151+
it('when generic type is specified and anything is passed', () => {
152+
expectSnippet(
153+
`componentStore.effect<void>((e) => EMPTY)(undefined);`
154+
).toFail(/Expected 0 arguments, but got 1/);
155+
});
156+
});
157+
});
158+
});
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
export const compilerOptions = () => ({
2+
moduleResolution: 'node',
3+
target: 'es2017',
4+
baseUrl: '.',
5+
experimentalDecorators: true,
6+
paths: {
7+
'@ngrx/component-store': ['./modules/component-store'],
8+
},
9+
});

modules/component-store/src/component-store.ts

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,6 @@ import {
2828
Inject,
2929
} from '@angular/core';
3030

31-
/**
32-
* Return type of the effect, that behaves differently based on whether the
33-
* argument is passed to the callback.
34-
*/
35-
export interface EffectReturnFn<T> {
36-
(): void;
37-
(t: T | Observable<T>): Subscription;
38-
}
39-
4031
export interface SelectConfig {
4132
debounce?: boolean;
4233
}
@@ -47,7 +38,7 @@ export const initialStateToken = new InjectionToken('ComponentStore InitState');
4738
export class ComponentStore<T extends object> implements OnDestroy {
4839
// Should be used only in ngOnDestroy.
4940
private readonly destroySubject$ = new ReplaySubject<void>(1);
50-
// Exposed to any extending Store to be used for the teardowns.
41+
// Exposed to any extending Store to be used for the teardown.
5142
readonly destroy$ = this.destroySubject$.asObservable();
5243

5344
private readonly stateSubject$ = new ReplaySubject<T>(1);
@@ -83,7 +74,7 @@ export class ComponentStore<T extends object> implements OnDestroy {
8374
* current state and an argument object) and returns a new instance of the
8475
* state.
8576
* @return A function that accepts one argument which is forwarded as the
86-
* second argument to `updaterFn`. Everytime this function is called
77+
* second argument to `updaterFn`. Every time this function is called
8778
* subscribers will be notified of the state change.
8879
*/
8980
updater<V>(
@@ -175,7 +166,7 @@ export class ComponentStore<T extends object> implements OnDestroy {
175166
*
176167
* @param projector A pure projection function that takes the current state and
177168
* returns some new slice/projection of that state.
178-
* @param config SelectConfig that changes the behavoir of selector, including
169+
* @param config SelectConfig that changes the behavior of selector, including
179170
* the debouncing of the values until the state is settled.
180171
* @return An observable of the projector results.
181172
*/
@@ -247,24 +238,39 @@ export class ComponentStore<T extends object> implements OnDestroy {
247238
* subscribed to for the life of the component.
248239
* @return A function that, when called, will trigger the origin Observable.
249240
*/
250-
effect<V, R = unknown>(
251-
generator: (origin$: Observable<V>) => Observable<R>
252-
): EffectReturnFn<V> {
253-
const origin$ = new Subject<V>();
254-
generator(origin$)
241+
effect<
242+
// This type quickly became part of effect 'API'
243+
ProvidedType = void,
244+
// The actual origin$ type, which could be unknown, when not specified
245+
OriginType extends Observable<ProvidedType> | unknown = Observable<
246+
ProvidedType
247+
>,
248+
// Unwrapped actual type of the origin$ Observable, after default was applied
249+
ObservableType = OriginType extends Observable<infer A> ? A : never,
250+
// Return either an empty callback or a function requiring specific types as inputs
251+
ReturnType = ProvidedType | ObservableType extends void
252+
? () => void
253+
: (
254+
observableOrValue: ObservableType | Observable<ObservableType>
255+
) => Subscription
256+
>(generator: (origin$: OriginType) => Observable<unknown>): ReturnType {
257+
const origin$ = new Subject<ObservableType>();
258+
generator(origin$ as OriginType)
255259
// tied to the lifecycle 👇 of ComponentStore
256260
.pipe(takeUntil(this.destroy$))
257261
.subscribe();
258262

259-
return (observableOrValue?: V | Observable<V>): Subscription => {
263+
return (((
264+
observableOrValue?: ObservableType | Observable<ObservableType>
265+
): Subscription => {
260266
const observable$ = isObservable(observableOrValue)
261267
? observableOrValue
262268
: of(observableOrValue);
263269
return observable$.pipe(takeUntil(this.destroy$)).subscribe((value) => {
264270
// any new 👇 value is pushed into a stream
265271
origin$.next(value);
266272
});
267-
};
273+
}) as unknown) as ReturnType;
268274
}
269275
}
270276

0 commit comments

Comments
 (0)