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: Pass optional param with all actions #1

Open
JonnyBGod opened this issue Feb 2, 2017 · 9 comments · May be fixed by #43
Open

Feat: Pass optional param with all actions #1

JonnyBGod opened this issue Feb 2, 2017 · 9 comments · May be fixed by #43
Assignees

Comments

@JonnyBGod
Copy link
Collaborator

This would provide an easy way to pass extra data for action extensibility.

Good example would be for an Alert reducer, enabling us to pass "success/fail" messages to be consumed by Alert Effects/Reducers.

This would work for other use cases if we keep optional param as open as possible.

@JonnyBGod JonnyBGod self-assigned this Feb 2, 2017
@JonnyBGod
Copy link
Collaborator Author

Major refactor though, will have to do it next week.

@brannon-darby
Copy link
Member

@JonnyBGod - if you can provide more details, i'll be happy to add this. i don't have much experience with ngrx though, so i may need some guidance.

@JonnyBGod
Copy link
Collaborator Author

I will try to update the ngrx implementation next week.

Did a few changes since this first version.

@JonnyBGod
Copy link
Collaborator Author

but for reference related to this issue it should be changed to something like this:

actions:

  actions.create = class implements Action {
    public readonly type = actionTypes['CREATE'];

    constructor(public payload: any, **public meta?: any**) { }
  };

effects:

  protected create: Observable<LoopbackAction> = this.actions$
    .ofType(this.actionTypes.CREATE)
    .mergeMap((action: LoopbackAction) =>
      this.apiService.create(action.payload)
        .map((response) => new this.actions.createSuccess(response, **action.meta**))
        .catch((error) => concat(
          of(new this.actions.createFail(error, **action.meta**)),
          of(new LoopbackErrorActions.error(error, **action.meta**))
        ))
    );

without the ** of course.

@JonnyBGod
Copy link
Collaborator Author

Another refactor should be the reducers helper methods.

https://github.com/mean-expert-official/fireloop-starter/blob/develop/webapp/src/app/shared/sdk/reducers/auth.ts#L67-L85

should change to

export const getLoopbackAuthState = (state: any) => state.loopbackAuth;
export const getLoopbackAuthToken = (state: any) => state.loopbackAuth;
export const getLoopbackAuthUser = (state: any) => state.loopbackAuth.user;
export const getLoopbackAuthUserId = (state: any) => state.loopbackAuth.userId;

and similar with other reducers.

@beeman
Copy link
Contributor

beeman commented Mar 25, 2017

@JonnyBGod good to hear you can spend some time on this soon. I hope to get this puppy out asap.

Let's hop on a call when you have time so we can iron out the things and talk through the steps we need to take to finalize.

@JonnyBGod
Copy link
Collaborator Author

also still related to this issue, for the meta parameter to validate with TS we have to add the following to BaseModels.ts

export interface LoopbackAction extends Action {
  meta?: any;
}

@JonnyBGod
Copy link
Collaborator Author

@beeman Yes, sure will get back to you for the call hopefully mid next week.

Sorry for almost two months of absence here but been busy. But I see you got a few extra hand on it 👍

@beeman
Copy link
Contributor

beeman commented Mar 25, 2017

Yeah the starter is doing great with help from @brannon-darby and @kattsushi. We aim to release a beta version once the ngrx stuff lands in the sdk-builder alpha/beta

@JonnyBGod JonnyBGod linked a pull request Apr 25, 2017 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants