diff --git a/modules/effects/spec/effect_decorator.spec.ts b/modules/effects/spec/effect_decorator.spec.ts deleted file mode 100644 index 80cf15bdde..0000000000 --- a/modules/effects/spec/effect_decorator.spec.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { Effect, getEffectDecoratorMetadata } from '../src/effect_decorator'; - -describe('@Effect()', () => { - describe('getEffectDecoratorMetadata', () => { - it('should get the effects metadata for a class instance', () => { - class Fixture { - @Effect() a: any; - @Effect({ dispatch: true }) - b: any; - @Effect({ dispatch: false }) - c: any; - @Effect({ useEffectsErrorHandler: true }) - d: any; - @Effect({ useEffectsErrorHandler: false }) - e: any; - @Effect({ dispatch: false, useEffectsErrorHandler: false }) - f: any; - @Effect({ dispatch: true, useEffectsErrorHandler: false }) - g: any; - } - - const mock = new Fixture(); - - expect(getEffectDecoratorMetadata(mock)).toEqual([ - { propertyName: 'a', dispatch: true, useEffectsErrorHandler: true }, - { propertyName: 'b', dispatch: true, useEffectsErrorHandler: true }, - { propertyName: 'c', dispatch: false, useEffectsErrorHandler: true }, - { propertyName: 'd', dispatch: true, useEffectsErrorHandler: true }, - { propertyName: 'e', dispatch: true, useEffectsErrorHandler: false }, - { propertyName: 'f', dispatch: false, useEffectsErrorHandler: false }, - { propertyName: 'g', dispatch: true, useEffectsErrorHandler: false }, - ]); - }); - - it('should return an empty array if the class has not been decorated', () => { - class Fixture { - a: any; - } - - const mock = new Fixture(); - - expect(getEffectDecoratorMetadata(mock)).toEqual([]); - }); - }); -}); diff --git a/modules/effects/spec/effect_sources.spec.ts b/modules/effects/spec/effect_sources.spec.ts index cb99d5afbc..3bf4464efe 100644 --- a/modules/effects/spec/effect_sources.spec.ts +++ b/modules/effects/spec/effect_sources.spec.ts @@ -13,7 +13,6 @@ import { import { map } from 'rxjs/operators'; import { - Effect, EffectSources, OnIdentifyEffects, OnInitEffects, @@ -76,251 +75,6 @@ describe('EffectSources', () => { return (effectSources as any)['toActions'].call(source); } - describe('with @Effect()', () => { - const a = { type: 'From Source A' }; - const b = { type: 'From Source B' }; - const c = { type: 'From Source C that completes' }; - const d = { not: 'a valid action' }; - const e = undefined; - const f = null; - const i = { type: 'From Source Identifier' }; - const i2 = { type: 'From Source Identifier 2' }; - - const circularRef = {} as any; - circularRef.circularRef = circularRef; - const g = { circularRef }; - - const error = new Error('An Error'); - - class SourceA { - @Effect() a$ = alwaysOf(a); - } - - class SourceB { - @Effect() b$ = alwaysOf(b); - } - - class SourceC { - @Effect() c$ = of(c); - } - - class SourceD { - @Effect() d$ = alwaysOf(d); - } - - class SourceE { - @Effect() e$ = alwaysOf(e); - } - - class SourceF { - @Effect() f$ = alwaysOf(f); - } - - class SourceG { - @Effect() g$ = alwaysOf(g); - } - - class SourceError { - @Effect() e$ = throwError(() => error); - } - - class SourceH { - @Effect() empty = of('value'); - @Effect() - never = timer(50, getTestScheduler() as any).pipe(map(() => 'update')); - } - - class SourceWithIdentifier implements OnIdentifyEffects { - effectIdentifier: string; - @Effect() i$ = alwaysOf(i); - - ngrxOnIdentifyEffects() { - return this.effectIdentifier; - } - - constructor(identifier: string) { - this.effectIdentifier = identifier; - } - } - - class SourceWithIdentifier2 implements OnIdentifyEffects { - effectIdentifier: string; - @Effect() i2$ = alwaysOf(i2); - - ngrxOnIdentifyEffects() { - return this.effectIdentifier; - } - - constructor(identifier: string) { - this.effectIdentifier = identifier; - } - } - - it('should resolve effects from instances', () => { - const sources$ = cold('--a--', { a: new SourceA() }); - const expected = cold('--a--', { a }); - - const output = toActions(sources$); - - expect(output).toBeObservable(expected); - }); - - it('should ignore duplicate sources', () => { - const sources$ = cold('--a--a--a--', { - a: new SourceA(), - }); - const expected = cold('--a--------', { a }); - - const output = toActions(sources$); - - expect(output).toBeObservable(expected); - }); - - it('should resolve effects with different identifiers', () => { - const sources$ = cold('--a--b--c--', { - a: new SourceWithIdentifier('a'), - b: new SourceWithIdentifier('b'), - c: new SourceWithIdentifier('c'), - }); - const expected = cold('--i--i--i--', { i }); - - const output = toActions(sources$); - - expect(output).toBeObservable(expected); - }); - - it('should ignore effects with the same identifier', () => { - const sources$ = cold('--a--b--c--', { - a: new SourceWithIdentifier('a'), - b: new SourceWithIdentifier('a'), - c: new SourceWithIdentifier('a'), - }); - const expected = cold('--i--------', { i }); - - const output = toActions(sources$); - - expect(output).toBeObservable(expected); - }); - - it('should resolve effects with same identifiers but different classes', () => { - const sources$ = cold('--a--b--c--d--', { - a: new SourceWithIdentifier('a'), - b: new SourceWithIdentifier2('a'), - c: new SourceWithIdentifier('b'), - d: new SourceWithIdentifier2('b'), - }); - const expected = cold('--a--b--a--b--', { a: i, b: i2 }); - - const output = toActions(sources$); - - expect(output).toBeObservable(expected); - }); - - it('should report an error if an effect dispatches an invalid action', () => { - const sources$ = of(new SourceD()); - - toActions(sources$).subscribe(); - - expect(mockErrorReporter.handleError).toHaveBeenCalledWith( - new Error( - 'Effect "SourceD.d$" dispatched an invalid action: {"not":"a valid action"}' - ) - ); - }); - - it('should report an error if an effect dispatches an `undefined`', () => { - const sources$ = of(new SourceE()); - - toActions(sources$).subscribe(); - - expect(mockErrorReporter.handleError).toHaveBeenCalledWith( - new Error( - 'Effect "SourceE.e$" dispatched an invalid action: undefined' - ) - ); - }); - - it('should report an error if an effect dispatches a `null`', () => { - const sources$ = of(new SourceF()); - - toActions(sources$).subscribe(); - - expect(mockErrorReporter.handleError).toHaveBeenCalledWith( - new Error('Effect "SourceF.f$" dispatched an invalid action: null') - ); - }); - - it('should report an error if an effect throws one', () => { - const sources$ = of(new SourceError()); - - toActions(sources$).subscribe(); - - expect(mockErrorReporter.handleError).toHaveBeenCalledWith( - new Error('An Error') - ); - }); - - it('should resubscribe on error by default', () => { - class Eff { - @Effect() - b$ = hot('a--e--b--e--c--e--d').pipe( - map((v) => { - if (v == 'e') throw new Error('An Error'); - return v; - }) - ); - } - - const sources$ = of(new Eff()); - - // 👇 'e' is ignored. - const expected = cold('a-----b-----c-----d'); - expect(toActions(sources$)).toBeObservable(expected); - }); - - it('should not resubscribe on error when useEffectsErrorHandler is false', () => { - class Eff { - @Effect({ useEffectsErrorHandler: false }) - b$ = hot('a--b--c--d').pipe( - map((v) => { - if (v == 'b') throw new Error('An Error'); - return v; - }) - ); - } - - const sources$ = of(new Eff()); - - // 👇 completes. - const expected = cold('a--|'); - - expect(toActions(sources$)).toBeObservable(expected); - }); - - it(`should not break when the action in the error message can't be stringified`, () => { - const sources$ = of(new SourceG()); - - toActions(sources$).subscribe(); - - expect(mockErrorReporter.handleError).toHaveBeenCalledWith( - new Error( - 'Effect "SourceG.g$" dispatched an invalid action: [object Object]' - ) - ); - }); - - it('should not complete the group if just one effect completes', () => { - const sources$ = cold('g', { - g: new SourceH(), - }); - const expected = cold('a----b-----', { a: 'value', b: 'update' }); - - const output = toActions(sources$); - - expect(output).toBeObservable(expected); - }); - }); - describe('with createEffect()', () => { const a = { type: 'From Source A' }; const b = { type: 'From Source B' }; diff --git a/modules/effects/spec/effects_feature_module.spec.ts b/modules/effects/spec/effects_feature_module.spec.ts index 7c426b4400..79ce8cc8cb 100644 --- a/modules/effects/spec/effects_feature_module.spec.ts +++ b/modules/effects/spec/effects_feature_module.spec.ts @@ -9,7 +9,7 @@ import { StoreModule, } from '@ngrx/store'; import { map, withLatestFrom } from 'rxjs/operators'; -import { Actions, Effect, EffectsModule, ofType, createEffect } from '../'; +import { Actions, EffectsModule, ofType, createEffect } from '../'; import { EffectsFeatureModule } from '../src/effects_feature_module'; import { EffectsRootModule } from '../src/effects_root_module'; import { FEATURE_EFFECTS } from '../src/tokens'; @@ -67,22 +67,6 @@ describe('Effects Feature Module', () => { store = TestBed.inject(Store); }); - it('should have the feature state defined to select from the effect', (done: any) => { - const action = { type: 'INCREMENT' }; - const result = { type: 'INCREASE' }; - - effects.effectWithStore.subscribe((res) => { - expect(res).toEqual(result); - }); - - store.dispatch(action); - - store.pipe(select(getDataState)).subscribe((data) => { - expect(data).toBe(110); - done(); - }); - }); - it('should have the feature state defined to select from the createEffect', (done: any) => { const action = { type: 'CREATE_INCREMENT' }; const result = { type: 'CREATE_INCREASE' }; @@ -145,13 +129,6 @@ const getCreateDataState = createSelector( class FeatureEffects { constructor(private actions: Actions, private store: Store) {} - @Effect() - effectWithStore = this.actions.pipe( - ofType('INCREMENT'), - withLatestFrom(this.store.select(getDataState)), - map(([action, state]) => ({ type: 'INCREASE' })) - ); - createEffectWithStore = createEffect(() => this.actions.pipe( ofType('CREATE_INCREMENT'), diff --git a/modules/effects/spec/effects_metadata.spec.ts b/modules/effects/spec/effects_metadata.spec.ts index 7ec7a878dd..a9bfe57cab 100644 --- a/modules/effects/spec/effects_metadata.spec.ts +++ b/modules/effects/spec/effects_metadata.spec.ts @@ -1,38 +1,49 @@ import { getEffectsMetadata, getSourceMetadata } from '../src/effects_metadata'; import { of } from 'rxjs'; -import { Effect, createEffect } from '..'; +import { createEffect } from '..'; import { EffectMetadata } from '../src/models'; describe('Effects metadata', () => { describe('getSourceMetadata', () => { - it('should combine effects created by the effect decorator and by createEffect', () => { + it('should create metadata for createEffect', () => { class Fixture { - @Effect() a: any; - b = createEffect(() => of({ type: 'a' })); - @Effect({ dispatch: false }) - c: any; - d = createEffect(() => of({ type: 'a' }), { dispatch: false }); - @Effect({ dispatch: false, useEffectsErrorHandler: false }) - e: any; - z: any; - f = createEffect(() => () => of({ type: 'a' })); - g = createEffect(() => () => of({ type: 'a' }), { dispatch: false }); - h = createEffect(() => () => of({ type: 'a' }), { - dispatch: true, - useEffectsErrorHandler: false, + effectSimple = createEffect(() => of({ type: 'a' })); + effectNoDispatch = createEffect(() => of({ type: 'a' }), { + dispatch: false, }); + noEffect: any; + effectWithMethod = createEffect(() => () => of({ type: 'a' })); + effectWithUseEffectsErrorHandler = createEffect( + () => () => of({ type: 'a' }), + { + dispatch: true, + useEffectsErrorHandler: false, + } + ); } const mock = new Fixture(); const expected: EffectMetadata[] = [ - { propertyName: 'a', dispatch: true, useEffectsErrorHandler: true }, - { propertyName: 'c', dispatch: false, useEffectsErrorHandler: true }, - { propertyName: 'b', dispatch: true, useEffectsErrorHandler: true }, - { propertyName: 'd', dispatch: false, useEffectsErrorHandler: true }, - { propertyName: 'e', dispatch: false, useEffectsErrorHandler: false }, - { propertyName: 'f', dispatch: true, useEffectsErrorHandler: true }, - { propertyName: 'g', dispatch: false, useEffectsErrorHandler: true }, - { propertyName: 'h', dispatch: true, useEffectsErrorHandler: false }, + { + propertyName: 'effectSimple', + dispatch: true, + useEffectsErrorHandler: true, + }, + { + propertyName: 'effectNoDispatch', + dispatch: false, + useEffectsErrorHandler: true, + }, + { + propertyName: 'effectWithMethod', + dispatch: true, + useEffectsErrorHandler: true, + }, + { + propertyName: 'effectWithUseEffectsErrorHandler', + dispatch: true, + useEffectsErrorHandler: false, + }, ]; expect(getSourceMetadata(mock)).toEqual( @@ -45,38 +56,40 @@ describe('Effects metadata', () => { describe('getEffectsMetadata', () => { it('should get map of metadata for all effects created', () => { class Fixture { - @Effect() a: any; - b = createEffect(() => of({ type: 'd' })); - @Effect({ dispatch: true }) - c: any; - d = createEffect(() => of({ type: 'e' }), { dispatch: true }); - @Effect({ dispatch: false }) - e: any; - f = createEffect(() => of({ type: 'f' }), { dispatch: false }); - g = createEffect(() => of({ type: 'g' }), { - useEffectsErrorHandler: false, - }); - h = createEffect(() => () => of({ type: 'a' })); - j = createEffect(() => () => of({ type: 'a' }), { dispatch: false }); - k = createEffect(() => () => of({ type: 'a' }), { - dispatch: true, - useEffectsErrorHandler: false, + effectSimple = createEffect(() => of({ type: 'a' })); + effectNoDispatch = createEffect(() => of({ type: 'a' }), { + dispatch: false, }); + noEffect: any; + effectWithMethod = createEffect(() => () => of({ type: 'a' })); + effectWithUseEffectsErrorHandler = createEffect( + () => () => of({ type: 'a' }), + { + dispatch: true, + useEffectsErrorHandler: false, + } + ); } const mock = new Fixture(); expect(getEffectsMetadata(mock)).toEqual({ - a: { dispatch: true, useEffectsErrorHandler: true }, - c: { dispatch: true, useEffectsErrorHandler: true }, - e: { dispatch: false, useEffectsErrorHandler: true }, - b: { dispatch: true, useEffectsErrorHandler: true }, - d: { dispatch: true, useEffectsErrorHandler: true }, - f: { dispatch: false, useEffectsErrorHandler: true }, - g: { dispatch: true, useEffectsErrorHandler: false }, - h: { dispatch: true, useEffectsErrorHandler: true }, - j: { dispatch: false, useEffectsErrorHandler: true }, - k: { dispatch: true, useEffectsErrorHandler: false }, + effectSimple: { + dispatch: true, + useEffectsErrorHandler: true, + }, + effectNoDispatch: { + dispatch: false, + useEffectsErrorHandler: true, + }, + effectWithMethod: { + dispatch: true, + useEffectsErrorHandler: true, + }, + effectWithUseEffectsErrorHandler: { + dispatch: true, + useEffectsErrorHandler: false, + }, }); }); diff --git a/modules/effects/src/effect_creator.ts b/modules/effects/src/effect_creator.ts index 6d52dfbfa4..ecbb8237b1 100644 --- a/modules/effects/src/effect_creator.ts +++ b/modules/effects/src/effect_creator.ts @@ -71,9 +71,9 @@ export function createEffect< return effect as typeof effect & CreateEffectMetadata; } -export function getCreateEffectMetadata< - T extends { [props in keyof T]: Object } ->(instance: T): EffectMetadata[] { +export function getCreateEffectMetadata>( + instance: T +): EffectMetadata[] { const propertyNames = Object.getOwnPropertyNames(instance) as Array; const metadata: EffectMetadata[] = propertyNames diff --git a/modules/effects/src/effect_decorator.ts b/modules/effects/src/effect_decorator.ts deleted file mode 100644 index 78993b7822..0000000000 --- a/modules/effects/src/effect_decorator.ts +++ /dev/null @@ -1,76 +0,0 @@ -import { compose } from '@ngrx/store'; - -import { - DEFAULT_EFFECT_CONFIG, - EffectConfig, - EffectMetadata, - EffectPropertyKey, -} from './models'; -import { getSourceForInstance } from './utils'; - -const METADATA_KEY = '__@ngrx/effects__'; - -/** - * @deprecated The Effect decorator (`@Effect`) is deprecated in favor for the `createEffect` method. - * See the docs for more info {@link https://ngrx.io/guide/migration/v11#the-effect-decorator} - */ -export function Effect(config: EffectConfig = {}) { - return function >( - target: T, - propertyName: K - ) { - const metadata: EffectMetadata = { - ...DEFAULT_EFFECT_CONFIG, - ...config, // Overrides any defaults if values are provided - propertyName, - }; - addEffectMetadataEntry(target, metadata); - }; -} - -export function getEffectDecoratorMetadata( - instance: T -): EffectMetadata[] { - const effectsDecorators: EffectMetadata[] = compose( - getEffectMetadataEntries, - getSourceForInstance - )(instance); - - return effectsDecorators; -} - -/** - * Type guard to detemine whether METADATA_KEY is already present on the Class - * constructor - */ -function hasMetadataEntries( - sourceProto: T -): sourceProto is typeof sourceProto & { - constructor: typeof sourceProto.constructor & { - [METADATA_KEY]: EffectMetadata[]; - }; -} { - return sourceProto.constructor.hasOwnProperty(METADATA_KEY); -} - -/** Add Effect Metadata to the Effect Class constructor under specific key */ -function addEffectMetadataEntry( - sourceProto: T, - metadata: EffectMetadata -) { - if (hasMetadataEntries(sourceProto)) { - sourceProto.constructor[METADATA_KEY].push(metadata); - } else { - Object.defineProperty(sourceProto.constructor, METADATA_KEY, { - value: [metadata], - }); - } -} - -function getEffectMetadataEntries( - sourceProto: T -): EffectMetadata[] { - return hasMetadataEntries(sourceProto) - ? sourceProto.constructor[METADATA_KEY] - : []; -} diff --git a/modules/effects/src/effects_metadata.ts b/modules/effects/src/effects_metadata.ts index a5704ae353..f53f053b47 100644 --- a/modules/effects/src/effects_metadata.ts +++ b/modules/effects/src/effects_metadata.ts @@ -1,8 +1,7 @@ import { EffectMetadata, EffectsMetadata } from './models'; import { getCreateEffectMetadata } from './effect_creator'; -import { getEffectDecoratorMetadata } from './effect_decorator'; -export function getEffectsMetadata( +export function getEffectsMetadata>( instance: T ): EffectsMetadata { return getSourceMetadata(instance).reduce( @@ -17,16 +16,8 @@ export function getEffectsMetadata( ); } -export function getSourceMetadata( +export function getSourceMetadata( instance: T ): EffectMetadata[] { - const effects: Array<(instance: Object) => EffectMetadata[]> = [ - getEffectDecoratorMetadata, - getCreateEffectMetadata, - ]; - - return effects.reduce[]>( - (sources, source) => sources.concat(source(instance)), - [] - ); + return getCreateEffectMetadata(instance); } diff --git a/modules/effects/src/index.ts b/modules/effects/src/index.ts index 5bc847ea89..c785303dfa 100644 --- a/modules/effects/src/index.ts +++ b/modules/effects/src/index.ts @@ -1,6 +1,5 @@ export { createEffect } from './effect_creator'; export { EffectConfig } from './models'; -export { Effect } from './effect_decorator'; export { getEffectsMetadata } from './effects_metadata'; export { mergeEffects } from './effects_resolver'; export { diff --git a/modules/effects/src/models.ts b/modules/effects/src/models.ts index 53b9c561d8..e36c8e2f7d 100644 --- a/modules/effects/src/models.ts +++ b/modules/effects/src/models.ts @@ -24,16 +24,16 @@ export interface CreateEffectMetadata { [CREATE_EFFECT_METADATA_KEY]: EffectConfig; } -export type EffectPropertyKey = Exclude< +export type EffectPropertyKey> = Exclude< keyof T, keyof Object >; -export interface EffectMetadata +export interface EffectMetadata> extends Required { propertyName: EffectPropertyKey; } -export type EffectsMetadata = { +export type EffectsMetadata> = { [Key in EffectPropertyKey]?: EffectConfig; }; diff --git a/projects/ngrx.io/content/guide/data/extension-points.md b/projects/ngrx.io/content/guide/data/extension-points.md index 5e130abfa2..5bc5141210 100644 --- a/projects/ngrx.io/content/guide/data/extension-points.md +++ b/projects/ngrx.io/content/guide/data/extension-points.md @@ -77,7 +77,7 @@ and re-setting `defaultDispatcherOptions` _before_ creating dispatchers ## Custom _effects_ -The NgRx Data library has one NgRx `@Effect`, the `EntityEffects` class. +The NgRx Data library has one NgRx `Effect`, the `EntityEffects` class. This class detects entity persistence actions, performs the persistence operation with a call to an `EntityDataService` and channels the HTTP response through a diff --git a/projects/ngrx.io/content/guide/effects/index.md b/projects/ngrx.io/content/guide/effects/index.md index 091a4f9e87..d329c80319 100644 --- a/projects/ngrx.io/content/guide/effects/index.md +++ b/projects/ngrx.io/content/guide/effects/index.md @@ -137,13 +137,6 @@ The `loadMovies$` effect is listening for all dispatched actions through the `Ac -
- -**Note:** You can also write effects using the `@Effect()` decorator, which was the previously defined way before effects creators were introduced in NgRx. If -you are looking for examples of effect decorators, visit the documentation for [versions 7.x and prior](https://v7.ngrx.io/guide/effects#writing-effects). - -
- ## Handling Errors Effects are built on top of observable streams provided by RxJS. Effects are listeners of observable streams that continue until an error or completion occurs. In order for effects to continue running in the event of an error in the observable, or completion of the observable stream, they must be nested within a "flattening" operator, such as `mergeMap`, `concatMap`, `exhaustMap` and other flattening operators. The example below shows the `loadMovies$` effect handling errors when fetching movies.