Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(store): add protection from type property use #1923

Merged
merged 2 commits into from
Jun 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions modules/store/spec/action_creator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ describe('Action Creators', () => {
const value = fooAction.bar;
`).toFail(/'bar' does not exist on type/);
});
it('should not allow type property', () => {
expectSnippet(`
const foo = createAction('FOO', (type: string) => ({type}));
`).toFail(
/Type '{ type: string; }' is not assignable to type '"type property is not allowed in action creators"/
);
});
});

describe('empty', () => {
Expand Down Expand Up @@ -141,5 +148,14 @@ describe('Action Creators', () => {
const value = fooAction.bar;
`).toFail(/'bar' does not exist on type/);
});

it('should not allow type property', () => {
const foo = createAction('FOO', props<{ type: number }>() as any);
expectSnippet(`
const foo = createAction('FOO', props<{ type: number }>());
`).toFail(
/Argument of type '"type property is not allowed in action creators"' is not assignable to parameter of type/
);
});
});
});
40 changes: 21 additions & 19 deletions modules/store/src/action_creator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,34 @@ import {
ActionCreator,
TypedAction,
FunctionWithParametersType,
ParametersType,
PropsReturnType,
DisallowTypeProperty,
} from './models';

/**
* Action creators taken from ts-action library and modified a bit to better
* fit current NgRx usage. Thank you Nicholas Jamieson (@cartant).
*/
// Action creators taken from ts-action library and modified a bit to better
// fit current NgRx usage. Thank you Nicholas Jamieson (@cartant).

export function createAction<T extends string>(
type: T
): ActionCreator<T, () => TypedAction<T>>;
export function createAction<T extends string, P extends object>(
type: T,
config: { _as: 'props'; _p: P }
): ActionCreator<T, (props: P) => P & TypedAction<T>>;
export function createAction<T extends string, C extends Creator>(
export function createAction<
T extends string,
P extends any[],
R extends object
>(
type: T,
creator: C
): FunctionWithParametersType<
ParametersType<C>,
ReturnType<C> & TypedAction<T>
> &
TypedAction<T>;
export function createAction<T extends string>(
creator: Creator<P, DisallowTypeProperty<R>>
): FunctionWithParametersType<P, R & TypedAction<T>> & TypedAction<T>;
export function createAction<T extends string, C extends Creator>(
type: T,
config?: { _as: 'props' } | Creator
config?: { _as: 'props' } | C
): Creator {
if (typeof config === 'function') {
return defineType(type, (...args: unknown[]) => ({
return defineType(type, (...args: any[]) => ({
...config(...args),
type,
}));
Expand All @@ -40,17 +40,19 @@ export function createAction<T extends string>(
case 'empty':
return defineType(type, () => ({ type }));
case 'props':
return defineType(type, (props: unknown) => ({
...(props as object),
return defineType(type, (props: object) => ({
...props,
type,
}));
default:
throw new Error('Unexpected config.');
}
}

export function props<P>(): { _as: 'props'; _p: P } {
return { _as: 'props', _p: undefined! };
export function props<P extends object>(): PropsReturnType<P> {
// the return type does not match TypePropertyIsNotAllowed, so double casting
// is used.
return ({ _as: 'props', _p: undefined! } as unknown) as PropsReturnType<P>;
}

export function union<
Expand Down
19 changes: 18 additions & 1 deletion modules/store/src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,24 @@ export type SelectorWithProps<State, Props, Result> = (
props: Props
) => Result;

export type Creator = (...args: any[]) => object;
export type DisallowTypeProperty<T> = T extends { type: any }
? TypePropertyIsNotAllowed
: T;

export const typePropertyIsNotAllowedMsg =
'type property is not allowed in action creators';
type TypePropertyIsNotAllowed = typeof typePropertyIsNotAllowedMsg;

export type Creator<
P extends any[] = any[],
R extends object = object
> = R extends { type: any }
? TypePropertyIsNotAllowed
: FunctionWithParametersType<P, R>;

export type PropsReturnType<T extends object> = T extends { type: any }
? TypePropertyIsNotAllowed
: { _as: 'props'; _p: T };

export type ActionCreator<
T extends string = string,
Expand Down