Skip to content

Commit

Permalink
$ refactor(effects): prepare for TS 3.5
Browse files Browse the repository at this point in the history
refactor(effects): Refactor types in effects, expose metadata key for createEffect
  • Loading branch information
alex-okrushko committed Nov 2, 2019
1 parent ccd3dd7 commit 1b70ff1
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 60 deletions.
8 changes: 4 additions & 4 deletions modules/effects/spec/effect_creator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ describe('createEffect()', () => {
});

it('should dispatch by default', () => {
const effect: any = createEffect(() => of({ type: 'a' }));
const effect = createEffect(() => of({ type: 'a' }));

expect(effect['__@ngrx/effects_create__']).toEqual(
jasmine.objectContaining({ dispatch: true })
);
});

it('should be possible to explicitly create a dispatching effect', () => {
const effect: any = createEffect(() => of({ type: 'a' }), {
const effect = createEffect(() => of({ type: 'a' }), {
dispatch: true,
});

Expand All @@ -27,7 +27,7 @@ describe('createEffect()', () => {
});

it('should be possible to create a non-dispatching effect', () => {
const effect: any = createEffect(() => of({ type: 'a' }), {
const effect = createEffect(() => of({ someProp: 'a' }), {
dispatch: false,
});

Expand All @@ -37,7 +37,7 @@ describe('createEffect()', () => {
});

it('should be possible to create a non-dispatching effect returning a non-action', () => {
const effect: any = createEffect(() => of('foo'), {
const effect = createEffect(() => of('foo'), {
dispatch: false,
});

Expand Down
25 changes: 12 additions & 13 deletions modules/effects/src/effect_creator.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import { Observable } from 'rxjs';
import { Action } from '@ngrx/store';
import { EffectMetadata, EffectConfig } from './models';
import { EffectMetadata, EffectConfig, DEFAULT_EFFECT_CONFIG } from './models';

const CREATE_EFFECT_METADATA_KEY = '__@ngrx/effects_create__';

interface CreateEffectMetadata {
[CREATE_EFFECT_METADATA_KEY]: EffectConfig;
}

type DispatchType<T> = T extends { dispatch: infer U } ? U : unknown;
type ObservableReturnType<T> = T extends false
? Observable<unknown>
Expand Down Expand Up @@ -45,27 +49,22 @@ export function createEffect<
T extends DispatchType<C>,
O extends ObservableReturnType<T>,
R extends O | ((...args: any[]) => O)
>(source: () => R, config?: Partial<C>): R {
>(source: () => R, config?: Partial<C>): R & CreateEffectMetadata {
const effect = source();
// Right now both createEffect and @Effect decorator set default values.
// Ideally that should only be done in one place that aggregates that info,
// for example in mergeEffects().
const value: EffectConfig = {
dispatch: true,
resubscribeOnError: true,
...DEFAULT_EFFECT_CONFIG,
...config, // Overrides any defaults if values are provided
};
Object.defineProperty(effect, CREATE_EFFECT_METADATA_KEY, {
value,
});
return effect;
return effect as typeof effect & CreateEffectMetadata;
}

export function getCreateEffectMetadata<T>(instance: T): EffectMetadata<T>[] {
const propertyNames = Object.getOwnPropertyNames(instance) as Extract<
keyof T,
string
>[];
export function getCreateEffectMetadata<
T extends { [props in keyof T]: Object }
>(instance: T): EffectMetadata<T>[] {
const propertyNames = Object.getOwnPropertyNames(instance) as Array<keyof T>;

const metadata: EffectMetadata<T>[] = propertyNames
.filter(
Expand Down
69 changes: 42 additions & 27 deletions modules/effects/src/effect_decorator.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
import { compose } from '@ngrx/store';
import { EffectMetadata, EffectConfig } from './models';

import {
DEFAULT_EFFECT_CONFIG,
EffectConfig,
EffectMetadata,
EffectPropertyKey,
} from './models';
import { getSourceForInstance } from './utils';

const METADATA_KEY = '__@ngrx/effects__';

export function Effect<T>({
dispatch = true,
resubscribeOnError = true,
}: EffectConfig = {}): PropertyDecorator {
return function<K extends Extract<keyof T, string>>(
export function Effect(config: EffectConfig = {}) {
return function<T extends Object, K extends EffectPropertyKey<T>>(
target: T,
propertyName: K
) {
// Right now both createEffect and @Effect decorator set default values.
// Ideally that should only be done in one place that aggregates that info,
// for example in mergeEffects().
const metadata: EffectMetadata<T> = {
...DEFAULT_EFFECT_CONFIG,
...config, // Overrides any defaults if values are provided
propertyName,
dispatch,
resubscribeOnError,
};
setEffectMetadataEntries<T>(target, [metadata]);
} as (target: {}, propertyName: string | symbol) => void;
addEffectMetadataEntry<T>(target, metadata);
};
}

export function getEffectDecoratorMetadata<T>(
Expand All @@ -35,23 +35,38 @@ export function getEffectDecoratorMetadata<T>(
return effectsDecorators;
}

function setEffectMetadataEntries<T>(
/**
* Type guard to detemine whether METADATA_KEY is already present on the Class
* constructor
*/
function hasMetadataEntries<T extends Object>(
sourceProto: T
): sourceProto is typeof sourceProto & {
constructor: typeof sourceProto.constructor & {
[METADATA_KEY]: EffectMetadata<T>[];
};
} {
return sourceProto.constructor.hasOwnProperty(METADATA_KEY);
}

/** Add Effect Metadata to the Effect Class constructor under specific key */
function addEffectMetadataEntry<T extends object>(
sourceProto: T,
entries: EffectMetadata<T>[]
metadata: EffectMetadata<T>
) {
const constructor = sourceProto.constructor;
const meta: Array<EffectMetadata<T>> = constructor.hasOwnProperty(
METADATA_KEY
)
? (constructor as any)[METADATA_KEY]
: Object.defineProperty(constructor, METADATA_KEY, { value: [] })[
METADATA_KEY
];
Array.prototype.push.apply(meta, entries);
if (hasMetadataEntries(sourceProto)) {
sourceProto.constructor[METADATA_KEY].push(metadata);
} else {
Object.defineProperty(sourceProto.constructor, METADATA_KEY, {
value: [metadata],
});
}
}

function getEffectMetadataEntries<T>(sourceProto: T): EffectMetadata<T>[] {
return sourceProto.constructor.hasOwnProperty(METADATA_KEY)
? (sourceProto.constructor as any)[METADATA_KEY]
function getEffectMetadataEntries<T extends object>(
sourceProto: T
): EffectMetadata<T>[] {
return hasMetadataEntries(sourceProto)
? sourceProto.constructor[METADATA_KEY]
: [];
}
4 changes: 2 additions & 2 deletions modules/effects/src/effect_notification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Notification, Observable } from 'rxjs';

export interface EffectNotification {
effect: Observable<any> | (() => Observable<any>);
propertyName: string;
propertyName: PropertyKey;
sourceName: string;
sourceInstance: any;
notification: Notification<Action | null | undefined>;
Expand Down Expand Up @@ -46,7 +46,7 @@ function getEffectName({
}: EffectNotification) {
const isMethod = typeof sourceInstance[propertyName] === 'function';

return `"${sourceName}.${propertyName}${isMethod ? '()' : ''}"`;
return `"${sourceName}.${String(propertyName)}${isMethod ? '()' : ''}"`;
}

function stringify(action: Action | null | undefined) {
Expand Down
21 changes: 10 additions & 11 deletions modules/effects/src/effects_metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,16 @@ import { getCreateEffectMetadata } from './effect_creator';
import { getEffectDecoratorMetadata } from './effect_decorator';

export function getEffectsMetadata<T>(instance: T): EffectsMetadata<T> {
const metadata: EffectsMetadata<T> = {};

for (const {
propertyName,
dispatch,
resubscribeOnError,
} of getSourceMetadata(instance)) {
metadata[propertyName] = { dispatch, resubscribeOnError };
}

return metadata;
return getSourceMetadata(instance).reduce(
(
acc: EffectsMetadata<T>,
{ propertyName, dispatch, resubscribeOnError }
) => {
acc[propertyName] = { dispatch, resubscribeOnError };
return acc;
},
{}
);
}

export function getSourceMetadata<T>(instance: T): EffectMetadata<T>[] {
Expand Down
17 changes: 14 additions & 3 deletions modules/effects/src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,21 @@ export interface EffectConfig {
resubscribeOnError?: boolean;
}

export interface EffectMetadata<T> extends Required<EffectConfig> {
propertyName: Extract<keyof T, string>;
export const DEFAULT_EFFECT_CONFIG: Required<EffectConfig> = {
dispatch: true,
resubscribeOnError: true,
};

export type EffectPropertyKey<T extends Object> = Exclude<
keyof T,
keyof Object
>;

export interface EffectMetadata<T extends Object>
extends Required<EffectConfig> {
propertyName: EffectPropertyKey<T>;
}

export type EffectsMetadata<T> = {
[key in Extract<keyof T, string>]?: EffectConfig
[key in EffectPropertyKey<T>]?: EffectConfig
};

0 comments on commit 1b70ff1

Please sign in to comment.