Skip to content
This repository has been archived by the owner on Jan 10, 2018. It is now read-only.

Provide option to send extra data #329

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JonnyBGod
Copy link

@JonnyBGod JonnyBGod commented Feb 3, 2017

Proposed feature:

Provide option to send extra data through optional meta field.

Why:

In many situation I need to pass extra data to be consumed by different Effects.

One example would be for an Alert reducer, enabling us to pass "success/fail" messages from any action.

Example flux:

  1. dispatch LOGIN action with payload and meta = {successMessage: "Welcome!"}.
  2. LOGIN effect passes meta to LOGIN_SUCCESS action after its own logic.
  3. ALERT reducer listening to LOGIN_SUCCESS changes its state: return action.meta.successMessage;

through optional meta field.
@JonnyBGod
Copy link
Author

JonnyBGod commented Feb 3, 2017

I realise I can achieve this easily by extending the Action interface.

Just proposing in case you think it makes sense to do it.

@littleStudent
Copy link

I dont get why you cant use the payload for your successMessage?

@JonnyBGod
Copy link
Author

I could use the payload, but then in every action, even the most common case that is where payload is composed by a single set of data, I would need to deal with mapping it.

Also not very nice to mix functional data with extras. It is actually a very bad practice to do so in my opinion.

@fxck
Copy link

fxck commented Feb 3, 2017

I actually think this would be handy.

@JonnyBGod
Copy link
Author

The idea come from need and reading https://github.com/acdlite/flux-standard-action.

@dballance
Copy link

Just curious, why can't you extend action in your app? I like the small footprint of @ngrx/store Actions - makes it flexible for you to implement Flux Standard Actions if you want, or some other standard action that implements your needs.

import { Action } from '@ngrx/store';

export interface FluxStandardAction extends Action {
    meta?: any;
    error: any;
}

Not saying this shouldn't be merged, just would prefer the smaller footprint from the library. If I don't use meta, I don't get it within my Action signature, but it an be extended if you need / use it.

@fxck
Copy link

fxck commented Feb 3, 2017

The problem is with effects.

@JonnyBGod
Copy link
Author

JonnyBGod commented Feb 3, 2017

@dballance I can, I mention that in my second entry. But I end up using it a lot and it seams to me that it should be here. But that is why I raised it for discussion. I can live with it as is but really think it would improve ngrx/store and also make it easier for new users.

@fxck what do you mean?

@fxck
Copy link

fxck commented Feb 3, 2017

You can't locally extend Action interface like that because @ngrx/effects use Action interface from @ngrx/store https://github.com/ngrx/effects/blob/master/src/actions.ts#L10

@abdulhaq-e
Copy link

I saw this in react-redux-forms. The code relies heavily on a meta property in addition to the action type and payload.

@dballance
Copy link

dballance commented Feb 3, 2017

@fxck - Make sense, but also makes me wonder. Would the better option not be a PR against @ngrx/effects for something like this? It's a breaking change, but shouldn't be to hard to overcome in any apps.

@Injectable()
export class Actions<T extends Action> extends Observable<T> {
  constructor(@Inject(Dispatcher) actionsSubject: Observable<T>) {
    super();
    this.source = actionsSubject;
  }

  lift(operator) {
    const observable = new Actions<T>(this);
    observable.operator = operator;
    return observable;
  }

  ofType(...keys: string[]): Actions<T> {
    return filter.call(this, ({ type }) => {
      const len = keys.length;
      if (len === 1) {
        return type === keys[0];
      } else {
        for (let i = 0; i < len; i++) {
          if (keys[i] === type) {
            return true;
          }
        }
      }
      return false;
    });
  }
}

Which has the effect of making this work

@Injectable()
export class AuthEffects {
  constructor(
    private http: Http,
    private actions$: Actions<FluxStandardAction>
  ) { }

  @Effect() stream$ = this.actions$
      .ofType('SOME_ACTION')
      .map(({type, payload, meta, error}) =>{
         // meta, error, payload, and type are all typed here
        // rest of code here
      })
}

@karolmie1
Copy link

@fxck You can extend actions with effects, you just have to make sure you have 1 to 1 mapping from string to class and then you can simply cast it to proper type.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants