From fc5af33bbc4e809d8a744a49992051ae7922ceac Mon Sep 17 00:00:00 2001 From: Alex Okrushko Date: Tue, 4 Aug 2020 22:34:06 -0400 Subject: [PATCH] refactor(component-store): fine-tune effect types --- .../spec/component-store.spec.ts | 26 +-- .../spec/types/component-store.types.spec.ts | 158 ++++++++++++++++++ modules/component-store/spec/types/utils.ts | 9 + .../component-store/src/component-store.ts | 44 ++--- 4 files changed, 205 insertions(+), 32 deletions(-) create mode 100644 modules/component-store/spec/types/component-store.types.spec.ts create mode 100644 modules/component-store/spec/types/utils.ts diff --git a/modules/component-store/spec/component-store.spec.ts b/modules/component-store/spec/component-store.spec.ts index 0751e97939..1f781534ba 100644 --- a/modules/component-store/spec/component-store.spec.ts +++ b/modules/component-store/spec/component-store.spec.ts @@ -425,7 +425,7 @@ describe('Component Store', () => { componentStore.state$.subscribe((state) => results.push(state)); // Update with Observable. - const subsription = updater( + const subscription = updater( interval(10).pipe( map((v) => ({ value: String(v) })), take(10) // just in case @@ -435,7 +435,7 @@ describe('Component Store', () => { // Advance for 40 fake milliseconds and unsubscribe - should capture // from '0' to '3' advance(40); - subsription.unsubscribe(); + subscription.unsubscribe(); // Advance for 20 more fake milliseconds, to check if anything else // is captured @@ -468,7 +468,7 @@ describe('Component Store', () => { componentStore.state$.subscribe((state) => results.push(state)); // Update with Observable. - const subsription = updater( + const subscription = updater( interval(10).pipe( map((v) => ({ value: 'a' + v })), take(10) // just in case @@ -486,7 +486,7 @@ describe('Component Store', () => { // Advance for 40 fake milliseconds and unsubscribe - should capture // from '0' to '3' advance(40); - subsription.unsubscribe(); + subscription.unsubscribe(); // Advance for 30 more fake milliseconds, to make sure that second // Observable still emits @@ -1119,7 +1119,7 @@ describe('Component Store', () => { origin$.pipe(tap((v) => results.push(typeof v))) ); const effect = componentStore.effect(mockGenerator); - effect(undefined); + effect(); effect(); expect(results).toEqual(['undefined', 'undefined']); @@ -1130,7 +1130,7 @@ describe('Component Store', () => { 'is run when observable is provided', marbles((m) => { const mockGenerator = jest.fn((origin$) => origin$); - const effect = componentStore.effect(mockGenerator); + const effect = componentStore.effect(mockGenerator); effect(m.cold('-a-b-c|')); @@ -1143,7 +1143,7 @@ describe('Component Store', () => { 'is run with multiple Observables', marbles((m) => { const mockGenerator = jest.fn((origin$) => origin$); - const effect = componentStore.effect(mockGenerator); + const effect = componentStore.effect(mockGenerator); effect(m.cold('-a-b-c|')); effect(m.hot(' --d--e----f-')); @@ -1170,12 +1170,12 @@ describe('Component Store', () => { ); // Update with Observable. - const subsription = effect(observable$); + const subscription = effect(observable$); // Advance for 40 fake milliseconds and unsubscribe - should capture // from '0' to '3' advance(40); - subsription.unsubscribe(); + subscription.unsubscribe(); // Advance for 20 more fake milliseconds, to check if anything else // is captured @@ -1196,7 +1196,7 @@ describe('Component Store', () => { ); // Pass the first Observable to the effect. - const subsription = effect( + const subscription = effect( interval(10).pipe( map((v) => ({ value: 'a' + v })), take(10) // just in case @@ -1214,7 +1214,7 @@ describe('Component Store', () => { // Advance for 40 fake milliseconds and unsubscribe - should capture // from '0' to '3' advance(40); - subsription.unsubscribe(); + subscription.unsubscribe(); // Advance for 30 more fake milliseconds, to make sure that second // Observable still emits @@ -1236,7 +1236,7 @@ describe('Component Store', () => { ); it('completes when componentStore is destroyed', (doneFn: jest.DoneCallback) => { - componentStore.effect((origin$) => + componentStore.effect((origin$: Observable) => origin$.pipe( finalize(() => { doneFn(); @@ -1249,7 +1249,7 @@ describe('Component Store', () => { }); it('observable argument completes when componentStore is destroyed', (doneFn: jest.DoneCallback) => { - componentStore.effect((origin$) => origin$)( + componentStore.effect((origin$: Observable) => origin$)( interval(10).pipe( finalize(() => { doneFn(); diff --git a/modules/component-store/spec/types/component-store.types.spec.ts b/modules/component-store/spec/types/component-store.types.spec.ts new file mode 100644 index 0000000000..77178bc509 --- /dev/null +++ b/modules/component-store/spec/types/component-store.types.spec.ts @@ -0,0 +1,158 @@ +import { expecter } from 'ts-snippet'; +import { compilerOptions } from './utils'; + +describe('ComponentStore types', () => { + describe('effect', () => { + const expectSnippet = expecter( + (code) => ` + import { ComponentStore } from '@ngrx/component-store'; + import { of, EMPTY, Observable } from 'rxjs'; + import { concatMap } from 'rxjs/operators'; + + const number$: Observable = of(5); + const string$: Observable = of('string'); + + const componentStore = new ComponentStore(); + ${code} + `, + compilerOptions() + ); + + describe('infers Subscription', () => { + it('when argument type is specified and a variable with corresponding type is passed', () => { + expectSnippet( + `const eff = componentStore.effect((e: Observable) => number$)('string');` + ).toInfer('eff', 'Subscription'); + }); + + it( + 'when argument type is specified, returns EMPTY and ' + + 'a variable with corresponding type is passed', + () => { + expectSnippet( + `const eff = componentStore.effect((e: Observable) => EMPTY)('string');` + ).toInfer('eff', 'Subscription'); + } + ); + + it('when argument type is specified and an Observable with corresponding type is passed', () => { + expectSnippet( + `const eff = componentStore.effect((e: Observable) => EMPTY)(string$);` + ).toInfer('eff', 'Subscription'); + }); + + it('when argument type is specified as Observable and any type is passed', () => { + expectSnippet( + `const eff = componentStore.effect((e: Observable) => EMPTY)(5);` + ).toInfer('eff', 'Subscription'); + }); + + it('when generic type is specified and a variable with corresponding type is passed', () => { + expectSnippet( + `const eff = componentStore.effect((e) => number$)('string');` + ).toInfer('eff', 'Subscription'); + }); + + it('when generic type is specified as unknown and a variable with any type is passed', () => { + expectSnippet( + `const eff = componentStore.effect((e) => number$)('string');` + ).toInfer('eff', 'Subscription'); + }); + + it('when generic type is specified as unknown and origin can still be piped', () => { + expectSnippet( + `const eff = componentStore.effect((e) => e.pipe(concatMap(() => of())))('string');` + ).toInfer('eff', 'Subscription'); + }); + + it('when generic type is specified as unknown and origin can still be piped', () => { + expectSnippet( + `const eff = componentStore.effect((e) => e.pipe(concatMap(() => of())))('string');` + ).toInfer('eff', 'Subscription'); + }); + }); + + describe('infers void', () => { + it('when argument type is specified as Observable and nothing is passed', () => { + expectSnippet( + `const eff = componentStore.effect((e: Observable) => string$)();` + ).toInfer('eff', 'void'); + }); + + it('when type is not specified and origin can still be piped', () => { + expectSnippet( + // treated as Observable 👇 + `const eff = componentStore.effect((e) => e.pipe(concatMap(() => of())))();` + ).toInfer('eff', 'void'); + }); + + it('when generic type is specified as void and origin can still be piped', () => { + expectSnippet( + `const eff = componentStore.effect((e) => e.pipe(concatMap(() => number$)))();` + ).toInfer('eff', 'void'); + }); + }); + + describe('catches improper usage', () => { + it('when type is specified and argument is not passed', () => { + expectSnippet( + `componentStore.effect((e: Observable) => of())();` + ).toFail(/Expected 1 arguments, but got 0/); + }); + + it('when type is specified and argument of incorrect type is passed', () => { + expectSnippet( + `componentStore.effect((e: Observable) => number$)(5);` + ).toFail( + /Argument of type '5' is not assignable to parameter of type 'string \| Observable'./ + ); + }); + + it('when type is specified and Observable argument of incorrect type is passed', () => { + expectSnippet( + `componentStore.effect((e: Observable) => string$)(number$);` + ).toFail( + /Argument of type 'Observable' is not assignable to parameter of type 'string \| Observable'/ + ); + }); + + it('when argument type is specified as Observable and type is not passed', () => { + expectSnippet( + `componentStore.effect((e: Observable) => EMPTY)();` + ).toFail(/Expected 1 arguments, but got 0/); + }); + + it('when generic type is specified and a variable with incorrect type is passed', () => { + expectSnippet( + `componentStore.effect((e) => number$)(5);` + ).toFail( + /Argument of type '5' is not assignable to parameter of type 'string \| Observable'/ + ); + }); + + it('when generic type is specified as unknown and a variable is not passed', () => { + expectSnippet( + `componentStore.effect((e) => number$)();` + ).toFail(/Expected 1 arguments, but got 0/); + }); + + it('when argument type is specified as Observable and anything is passed', () => { + expectSnippet( + `componentStore.effect((e: Observable) => string$)(5);` + ).toFail(/Expected 0 arguments, but got 1/); + }); + + it('when type is not specified and anything is passed', () => { + expectSnippet( + `const eff = componentStore.effect((e) => EMPTY)('string');` + ).toFail(/Expected 0 arguments, but got 1/); + }); + + it('when generic type is specified and anything is passed', () => { + expectSnippet( + `componentStore.effect((e) => EMPTY)(undefined);` + ).toFail(/Expected 0 arguments, but got 1/); + }); + }); + }); +}); diff --git a/modules/component-store/spec/types/utils.ts b/modules/component-store/spec/types/utils.ts new file mode 100644 index 0000000000..f0d19ca56a --- /dev/null +++ b/modules/component-store/spec/types/utils.ts @@ -0,0 +1,9 @@ +export const compilerOptions = () => ({ + moduleResolution: 'node', + target: 'es2017', + baseUrl: '.', + experimentalDecorators: true, + paths: { + '@ngrx/component-store': ['./modules/component-store'], + }, +}); diff --git a/modules/component-store/src/component-store.ts b/modules/component-store/src/component-store.ts index e02bcf3786..d311059681 100644 --- a/modules/component-store/src/component-store.ts +++ b/modules/component-store/src/component-store.ts @@ -28,15 +28,6 @@ import { Inject, } from '@angular/core'; -/** - * Return type of the effect, that behaves differently based on whether the - * argument is passed to the callback. - */ -export interface EffectReturnFn { - (): void; - (t: T | Observable): Subscription; -} - export interface SelectConfig { debounce?: boolean; } @@ -47,7 +38,7 @@ export const initialStateToken = new InjectionToken('ComponentStore InitState'); export class ComponentStore implements OnDestroy { // Should be used only in ngOnDestroy. private readonly destroySubject$ = new ReplaySubject(1); - // Exposed to any extending Store to be used for the teardowns. + // Exposed to any extending Store to be used for the teardown. readonly destroy$ = this.destroySubject$.asObservable(); private readonly stateSubject$ = new ReplaySubject(1); @@ -83,7 +74,7 @@ export class ComponentStore implements OnDestroy { * current state and an argument object) and returns a new instance of the * state. * @return A function that accepts one argument which is forwarded as the - * second argument to `updaterFn`. Everytime this function is called + * second argument to `updaterFn`. Every time this function is called * subscribers will be notified of the state change. */ updater( @@ -175,7 +166,7 @@ export class ComponentStore implements OnDestroy { * * @param projector A pure projection function that takes the current state and * returns some new slice/projection of that state. - * @param config SelectConfig that changes the behavoir of selector, including + * @param config SelectConfig that changes the behavior of selector, including * the debouncing of the values until the state is settled. * @return An observable of the projector results. */ @@ -247,16 +238,31 @@ export class ComponentStore implements OnDestroy { * subscribed to for the life of the component. * @return A function that, when called, will trigger the origin Observable. */ - effect( - generator: (origin$: Observable) => Observable - ): EffectReturnFn { - const origin$ = new Subject(); - generator(origin$) + effect< + // This type quickly became part of effect 'API' + ProvidedType = void, + // The actual origin$ type, which could be unknown, when not specified + OriginType extends Observable | unknown = Observable< + ProvidedType + >, + // Unwrapped actual type of the origin$ Observable, after default was applied + ObservableType = OriginType extends Observable ? A : never, + // Return either an empty callback or a function requiring specific types as inputs + ReturnType = ProvidedType | ObservableType extends void + ? () => void + : ( + observableOrValue: ObservableType | Observable + ) => Subscription + >(generator: (origin$: OriginType) => Observable): ReturnType { + const origin$ = new Subject(); + generator(origin$ as OriginType) // tied to the lifecycle 👇 of ComponentStore .pipe(takeUntil(this.destroy$)) .subscribe(); - return (observableOrValue?: V | Observable): Subscription => { + return ((( + observableOrValue?: ObservableType | Observable + ): Subscription => { const observable$ = isObservable(observableOrValue) ? observableOrValue : of(observableOrValue); @@ -264,7 +270,7 @@ export class ComponentStore implements OnDestroy { // any new 👇 value is pushed into a stream origin$.next(value); }); - }; + }) as unknown) as ReturnType; } }