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 action creators #1570

Closed
wants to merge 2 commits into from
Closed
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
122 changes: 122 additions & 0 deletions modules/store/spec/action_creator.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import { ActionsUnion, createAction } from '@ngrx/store';

describe('createAction with properties', () => {
describe('action type: const', () => {
it('type only', () => {
const ACTION1 = '[action] Action1 blah';
const actualAction = createAction(ACTION1);
type expectedActionType = Readonly<{
type: '[action] Action1 blah';
}>;

const expectedAction: expectedActionType = {
type: ACTION1,
};
expect(actualAction).toEqual(expectedAction);
});

it('type and properties', () => {
const properties = { userName: 'UserName', age: 32 };
const ACTION1 = '[action] Action1 blah';
const actualAction = createAction(ACTION1, properties);
type expectedActionType = Readonly<{
type: '[action] Action1 blah';
userName: string;
age: number;
}>;

const expectedAction: expectedActionType = {
type: ACTION1,
userName: properties.userName,
age: properties.age,
};
expect(actualAction).toEqual(expectedAction);
});
});

describe('action type: enum', () => {
it('type only', () => {
enum ActionTypes {
Action1 = '[action] Action1 blah',
Action2 = '[action] Action2 blah',
}

const actualAction = createAction(ActionTypes.Action1);
type expectedActionType = Readonly<{
type: ActionTypes.Action1;
}>;

const expectedAction: expectedActionType = {
type: ActionTypes.Action1,
};
expect(actualAction).toEqual(expectedAction);
});

it('type and properties', () => {
const properties = { userName: 'UserName', age: 32 };
enum ActionTypes {
Action1 = '[action] Action1 blah',
Action2 = '[action] Action2 blah',
}

const actualAction = createAction(ActionTypes.Action1, properties);
type expectedActionType = Readonly<{
type: ActionTypes.Action1;
userName: string;
age: number;
}>;

const expectedAction: expectedActionType = {
type: ActionTypes.Action1,
userName: properties.userName,
age: properties.age,
};
expect(actualAction).toEqual(expectedAction);
});
});
});

describe('ActionsUnion type', () => {
enum ActionTypes {
Login = '[Login Page] Login',
Logout = '[Login Page] Logout',
}

const Actions = {
login: (userName: string, password: string) =>
createAction(ActionTypes.Login, { userName, password }),
logout: () => createAction(ActionTypes.Logout),
};

type Actions = ActionsUnion<typeof Actions>;

it('Action with properties', () => {
const properties = { userName: 'UserName', password: 'Password' };
const actualAction = Actions.login(
properties.userName,
properties.password
);
type expectedActionType = Readonly<{
type: ActionTypes.Login;
userName: string;
password: string;
}>;
const expectedAction: expectedActionType = {
type: ActionTypes.Login,
userName: properties.userName,
password: properties.password,
};
expect(actualAction).toEqual(expectedAction);
});

it('Action without properties', () => {
const actualAction = Actions.logout();
type expectedActionType = Readonly<{
type: ActionTypes.Logout;
}>;
const expectedAction: expectedActionType = {
type: ActionTypes.Logout,
};
expect(actualAction).toEqual(expectedAction);
});
});
39 changes: 39 additions & 0 deletions modules/store/src/action_creator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// ==========
// = Adapted from: rex-tils
// = https://github.com/Hotell/rex-tils
// ==========

import { Action, AnyFn } from './models';

export function createAction<T extends string>(type: T): Action<T>;
export function createAction<
T extends string,
P extends {
[key: string]: any;
}
>(type: T, props: P): Action<T, P>;
export function createAction<
T extends string,
P extends {
[key: string]: any;
}
>(type: T, props?: P) {
/*
The following line requires Typescript 3.2: Generic spread expressions in
object literals.
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-2.html

Typescript 3.1.1 gives error: Spread types may only be created from
object types. ts(2698)
*/
// const action = payload === undefined ?{ type } : { type, ...payload };
const action =
props === undefined ? { type } : { type, ...(props as object) };
return action;
}

/**
* Simple alias to save keystrokes when defining JS typed object maps
*/
type StringMap<T> = { [key: string]: T };
export type ActionsUnion<A extends StringMap<AnyFn>> = ReturnType<A[keyof A]>;
1 change: 1 addition & 0 deletions modules/store/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,4 @@ export {
_createStoreReducers,
_createFeatureReducers,
} from './store_module';
export { ActionsUnion, createAction } from './action_creator';
11 changes: 8 additions & 3 deletions modules/store/src/models.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
export interface Action {
type: string;
}
export type Action<T extends string = string, P = void> = P extends void
? Readonly<{ type: T }>
: Readonly<{
type: T;
}> &
Readonly<P>;

export type AnyFn = (...args: any[]) => any;

export type TypeId<T> = () => T;

Expand Down
4 changes: 1 addition & 3 deletions modules/store/src/selector.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { Selector, SelectorWithProps } from './models';

export type AnyFn = (...args: any[]) => any;
import { AnyFn, Selector, SelectorWithProps } from './models';

export type MemoizedProjection = { memoized: AnyFn; reset: () => void };

Expand Down
26 changes: 9 additions & 17 deletions projects/example-app/src/app/auth/actions/auth-api.actions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Action } from '@ngrx/store';
import { ActionsUnion, createAction } from '@ngrx/store';
import { User } from '@example-app/auth/models/user';

export enum AuthApiActionTypes {
Expand All @@ -7,20 +7,12 @@ export enum AuthApiActionTypes {
LoginRedirect = '[Auth/API] Login Redirect',
}

export class LoginSuccess implements Action {
readonly type = AuthApiActionTypes.LoginSuccess;
export const AuthApiActions = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having functions directly in the file would be better. You can then use named module import as a grouping mechanism.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to group them this way to take advantage of the ActionsUnion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So a few issues I have with this:

  • both function and classes Action creator are allowed and welcomed. This changes to function-specific.
  • according to Google's TS style guide:

When providing a structural-based implementation, explicitly include the type at the declaration of the symbol (this allows more precise type checking and error reporting).

// use
const foo: Foo = {
  a: 123,
  b: 'abc',
}
// instead of
const foo = {
  a: 123,
  b: 'abc',
}

On the other hand, I don't think anyone at Google is using function-based action creator and this proposal could be fine.

I'm planning to propose something like this for the class-based action creators: https://www.typescriptlang.org/play/index.html#src=%2F%2F%20action_helper.ts%0D%0Ainterface%20Type%3CC%2C%20T%20extends%20string%3E%20%7B%0D%0A%20%20%20new%20(...args%3A%20any%5B%5D)%3A%20C%3B%0D%0A%20%20%20readonly%20type%3A%20T%3B%0D%0A%7D%0D%0Aabstract%20class%20StaticAction%3CT%20extends%20string%3E%20%7B%0D%0A%20%20static%20readonly%20type%3A%20string%3B%0D%0A%20%20readonly%20type!%3A%20T%3B%0D%0A%7D%0D%0A%0D%0Afunction%20Action%3CT%20extends%20string%3E(type%3A%20T)%3A%20Type%3CStaticAction%3CT%3E%2C%20T%3E%20%7B%0D%0A%20%20return%20class%20extends%20StaticAction%3CT%3E%20%7B%0D%0A%20%20%20%20readonly%20type%20%3D%20type%3B%0D%0A%20%20%20%20static%20type%20%3D%20type%3B%0D%0A%20%20%7D%0D%0A%7D%0D%0A%0D%0Aclass%20MyAction%20extends%20Action('foo')%20%7B%7D%0D%0A%0D%0Aconsole.log(new%20MyAction().type)%3B%20%2F%2F%20logs%3A%20'foo'%0D%0Aconsole.log(MyAction.type)%3B%20%2F%2F%20logs%3A%20'foo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should add that this was @kolodny idea, that I iterated on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will you open a new issue with this proposal? I'd lean towards creating a new API such as withAction('type') for this as opposed to changing Action.

loginRedirect: () => createAction(AuthApiActionTypes.LoginRedirect),
loginSuccess: (user: User) =>
createAction(AuthApiActionTypes.LoginSuccess, { user }),
loginFailure: (error: any) =>
createAction(AuthApiActionTypes.LoginFailure, { error }),
};

constructor(public payload: { user: User }) {}
}

export class LoginFailure implements Action {
readonly type = AuthApiActionTypes.LoginFailure;

constructor(public payload: { error: any }) {}
}

export class LoginRedirect implements Action {
readonly type = AuthApiActionTypes.LoginRedirect;
}

export type AuthApiActionsUnion = LoginSuccess | LoginFailure | LoginRedirect;
export type AuthApiActions = ActionsUnion<typeof AuthApiActions>;
24 changes: 8 additions & 16 deletions projects/example-app/src/app/auth/actions/auth.actions.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,16 @@
import { Action } from '@ngrx/store';
import { Action, ActionsUnion, createAction } from '@ngrx/store';

export enum AuthActionTypes {
Logout = '[Auth] Logout',
LogoutConfirmation = '[Auth] Logout Confirmation',
LogoutConfirmationDismiss = '[Auth] Logout Confirmation Dismiss',
}

export class Logout implements Action {
readonly type = AuthActionTypes.Logout;
}

export class LogoutConfirmation implements Action {
readonly type = AuthActionTypes.LogoutConfirmation;
}

export class LogoutConfirmationDismiss implements Action {
readonly type = AuthActionTypes.LogoutConfirmationDismiss;
}
export const AuthActions = {
logout: () => createAction(AuthActionTypes.Logout),
logoutConfirmation: () => createAction(AuthActionTypes.LogoutConfirmation),
logoutConfirmationDismiss: () =>
createAction(AuthActionTypes.LogoutConfirmationDismiss),
};

export type AuthActionsUnion =
| Logout
| LogoutConfirmation
| LogoutConfirmationDismiss;
export type AuthActions = ActionsUnion<typeof AuthActions>;
15 changes: 11 additions & 4 deletions projects/example-app/src/app/auth/actions/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import * as AuthActions from './auth.actions';
import * as AuthApiActions from './auth-api.actions';
import * as LoginPageActions from './login-page.actions';
import { AuthActions, AuthActionTypes } from './auth.actions';
import { AuthApiActions, AuthApiActionTypes } from './auth-api.actions';
import { LoginPageActions, LoginPageActionTypes } from './login-page.actions';

export { AuthActions, AuthApiActions, LoginPageActions };
export {
AuthActions,
AuthActionTypes,
AuthApiActions,
AuthApiActionTypes,
LoginPageActions,
LoginPageActionTypes,
};
13 changes: 6 additions & 7 deletions projects/example-app/src/app/auth/actions/login-page.actions.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { Action } from '@ngrx/store';
import { ActionsUnion, createAction } from '@ngrx/store';
import { Credentials } from '@example-app/auth/models/user';

export enum LoginPageActionTypes {
Login = '[Login Page] Login',
}

export class Login implements Action {
readonly type = LoginPageActionTypes.Login;
export const LoginPageActions = {
login: (credentials: Credentials) =>
createAction(LoginPageActionTypes.Login, { credentials }),
};

constructor(public payload: { credentials: Credentials }) {}
}

export type LoginPageActionsUnion = Login;
export type LoginPageActions = ActionsUnion<typeof LoginPageActions>;
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('Login Page', () => {

it('should dispatch a login event on submit', () => {
const credentials: any = {};
const action = new LoginPageActions.Login({ credentials });
const action = LoginPageActions.login(credentials);

instance.onSubmit(credentials);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ export class LoginPageComponent implements OnInit {
ngOnInit() {}

onSubmit(credentials: Credentials) {
this.store.dispatch(new LoginPageActions.Login({ credentials }));
this.store.dispatch(LoginPageActions.login(credentials));
}
}
26 changes: 13 additions & 13 deletions projects/example-app/src/app/auth/effects/auth.effects.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ describe('AuthEffects', () => {
it('should return an auth.LoginSuccess action, with user information if login succeeds', () => {
const credentials: Credentials = { username: 'test', password: '' };
const user = { name: 'User' } as User;
const action = new LoginPageActions.Login({ credentials });
const completion = new AuthApiActions.LoginSuccess({ user });
const action = LoginPageActions.login(credentials);
const completion = AuthApiActions.loginSuccess(user);

actions$ = hot('-a---', { a: action });
const response = cold('-a|', { a: user });
Expand All @@ -70,10 +70,10 @@ describe('AuthEffects', () => {

it('should return a new auth.LoginFailure if the login service throws', () => {
const credentials: Credentials = { username: 'someOne', password: '' };
const action = new LoginPageActions.Login({ credentials });
const completion = new AuthApiActions.LoginFailure({
error: 'Invalid username or password',
});
const action = LoginPageActions.login(credentials);
const completion = AuthApiActions.loginFailure(
'Invalid username or password'
);
const error = 'Invalid username or password';

actions$ = hot('-a---', { a: action });
Expand All @@ -88,7 +88,7 @@ describe('AuthEffects', () => {
describe('loginSuccess$', () => {
it('should dispatch a RouterNavigation action', (done: any) => {
const user = { name: 'User' } as User;
const action = new AuthApiActions.LoginSuccess({ user });
const action = AuthApiActions.loginSuccess(user);

actions$ = of(action);

Expand All @@ -101,7 +101,7 @@ describe('AuthEffects', () => {

describe('loginRedirect$', () => {
it('should dispatch a RouterNavigation action when auth.LoginRedirect is dispatched', (done: any) => {
const action = new AuthApiActions.LoginRedirect();
const action = AuthApiActions.loginRedirect();

actions$ = of(action);

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

it('should dispatch a RouterNavigation action when auth.Logout is dispatched', (done: any) => {
const action = new AuthActions.Logout();
const action = AuthActions.logout();

actions$ = of(action);

Expand All @@ -125,8 +125,8 @@ describe('AuthEffects', () => {

describe('logoutConfirmation$', () => {
it('should dispatch a Logout action if dialog closes with true result', () => {
const action = new AuthActions.LogoutConfirmation();
const completion = new AuthActions.Logout();
const action = AuthActions.logoutConfirmation();
const completion = AuthActions.logout();

actions$ = hot('-a', { a: action });
const expected = cold('-b', { b: completion });
Expand All @@ -139,8 +139,8 @@ describe('AuthEffects', () => {
});

it('should dispatch a LogoutConfirmationDismiss action if dialog closes with falsy result', () => {
const action = new AuthActions.LogoutConfirmation();
const completion = new AuthActions.LogoutConfirmationDismiss();
const action = AuthActions.logoutConfirmation();
const completion = AuthActions.logoutConfirmationDismiss();

actions$ = hot('-a', { a: action });
const expected = cold('-b', { b: completion });
Expand Down
Loading