ngrx / store

RxJS powered state management for Angular applications, inspired by Redux
MIT License
3.9k stars 310 forks source link

"@State" decorator for selecting state from Store #310

Closed orizens closed 7 years ago

orizens commented 7 years ago

[UPDATED 1/10/2017 8:28pm]
First, ngrx project is awesome - I use as the default state management with ng2 apps. After working a lot with ngrx/store on various projects, I was trying to think of a way to reduce boilerplate code for selecting reducers from the main store. This code is repeating itself, and I wish there was something shorter. i.e,:

export class YoutubeVideosComponent implements OnInit {
  videos$: Observable<EchoesVideos>;

  constructor(private store: Store<EchoesState>){}

  ngOnInit() {
    this.videos$ = this.store.select(state => state.videos);
  }
}

the code above might be declared with "@State" (or @Select) - so it is responsible to use the store instance within this component: or rather using "selectors":

import { getVideos$ } from '../reducers/videos.reducer.ts';

export class YoutubeVideosComponent implements OnInit {
  @State(getVideos$) videos$;
}

or strings (which might result in "magic"):

export class YoutubeVideosComponent implements OnInit {
  @State('videos') videos$: Observable<EchoesVideos>;
}

In the case where store for feature modules are supported - as discussed in @MikeRyan52 gist , code might be:

export class YoutubeVideosComponent implements OnInit {
  @State({
    // or - select: getVideos
    select: getVideos$,
    // or  FeatureStore can be provided as the Store
    store: FeatureStore
  })
  videos$: Observable<EchoesVideos>;
}

Thanks.

fxck commented 7 years ago

I honestly think this is the least painful boilerplate there is. I'm not even sure it would be possible, since the decorator would have to mess with DI(think AOT). The whole action(trigger) -> reducer(set loading state) -> effect(handle async) -> action(success) -> reducer(fill in data) is so much more painful.

import { getVideos$ } from '../reducers/videos.reducer.ts';

export class YoutubeVideosComponent {
  // no need for Observable<EchoesVideos> if you correctly typed your EchoesState 
  videos$ = this.store.let(getVideos$()); 

  // you'll most likely have a ctor with other DI anyway
  constructor(private store: Store<EchoesState>){} 
}

v.

import { getVideos$ } from '../reducers/videos.reducer.ts';

export class YoutubeVideosComponent {
  @State(getVideos$) videos$: Observable<EchoesVideos>;
}

is nearly the same and much less magic.

kylecordes commented 7 years ago

I like this more than a new feature.

orizens commented 7 years ago

hi @fxck - thanks for the suggestion - your code does seems to be minimal - you need to update to "this.store". Nevertheless, i'm always looking to express more in less code. my suggestion perhaps can be reduced to this without the need to inject the store in the ctor:

import { getVideos$ } from '../reducers/videos.reducer.ts';

export class YoutubeVideosComponent {
  @State(getVideos$) videos$;
}
fxck commented 7 years ago

Again, the biggest problem I see with this is that I'd have to be magic, you'd have to use hacks to be able to do DI in decorators. And hacks tend to lead to test and aot problems. Very little gain for a lot of potential problems.

I'm all for reducing boilerplate(I feel that typescript 2.1 could help with that), just not like this.

shlomiassaf commented 7 years ago

@fxck We can get the store without complex DI stuff... assuming there is 1 store

If there are multiple we can allow providing the store

orizens commented 7 years ago

Thanks to the kind help of @shlomiassaf, we managed to implement "@State(selectorFunc)": That's rather a good starting point and it works for one or multiple store registrations.

import { APP_BOOTSTRAP_LISTENER } from '@angular/core';
import { Store } from '@ngrx/store';
let _store;

export const BOOTSTRAP_TO_ACTION_PROVIDER = {
  provide: APP_BOOTSTRAP_LISTENER,
  multi: true,
  deps: [ Store ],
  useFactory: function (s) {
    _store = s;
    return (store) => store;
  }
};

export function State(selectorFunc: Function) {
  return function (target: any, key: string) {
    let _oldInit;
    if (target.ngOnInit) {
      _oldInit = target.ngOnInit;
    }
    target.ngOnInit = function(...args) {
      if (_store) {
        this[key] = _store.let(selectorFunc);
        if (_oldInit) {
          _oldInit.apply(this, args);
        }
      }
    };
  };
}

Working example:

export class YoutubeVideosComponent implements OnInit {
  @State(getVideos$) videos$;
  @State(getPlayerSearch$) playerSearch$;
}

however, a smarter way would be to save pair of "selectorFunc, key" pair target and initialize it in one ngOnInit. I'm aware that currently saving the old ngOnInit seems to be hacky - perhaps it can be enhanced. your comments are most welcomed.

Mark-McCracken commented 7 years ago

This looks really good. Would make using the library more approachable and the Components easier to read

hccampos commented 7 years ago

This would be pretty cool indeed. Maybe this technique could also be used to provide a "scoped" store which only has access to a slice of the state and which automatically dispatches actions with a certain namespace. Reusable, maintainable and readable components ftw!

fxck commented 7 years ago

I'm confused. What do you all find appealing on this? Sure enough it saves you having to do..

constructor(private _store: Store<AppState>) {}

..but other than that? Say you wanted to dispatch an action the store, or subscribe to your action stream and next it to the store, what would you do then? Inject store anyway?

hccampos commented 7 years ago

Dispach decorator for component methods? ^^

On Jan 11, 2017 08:20, "Aleš" notifications@github.com wrote:

I'm confused. What do you all find appealing on this? Sure enough it saves you having to do..

constructor(private _store: Store) {}

..but other than that? Say you wanted to dispatch an action the store, or subscribe to your action stream and next it to the store, what would you do then? Inject store anyway?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ngrx/store/issues/310#issuecomment-271796530, or mute the thread https://github.com/notifications/unsubscribe-auth/ADlcCr1OaGLufAJX_HL4eNjp2ExC8nniks5rRIKvgaJpZM4LffQs .

alexciesielski commented 7 years ago

Out of all the boilerplate there is, you want to reduce the most rewarding part of it (connecting everything to the view) ... ?

orizens commented 7 years ago

@MikeRyan52 is it possible to get a reference to the store in a decorator?

brandonroberts commented 7 years ago

@orizens decorators don't support DI, so not unless you use a workaround/hack to get an instance

orizens commented 7 years ago

thanks - i'll withdraw this suggestion for now.

flackjap commented 7 years ago

I just stumbled upon this by searching for a way to reduce boilerplate for store selectors. The proposed idea by @orizens is nice, but that's not actually what I was looking for.

My problem is that In some cases I just need to send some data to a 3rd party service. That data is obviously stored in the Store and I need to write selectors, subscribe to them and assign data to new variables before I call the 3rd party API. If I have 3 different properties that are not close to each other in a State object, I simply need to assign them to 3 local variables before sending them to a 3rd party API and that implies that I need to write 3 store.select.subscribe combos that usually take 4-5 lines each and I am left with 20~30 LoC just for the sake of grabbing some data from a Store object. I guess I could reduce this by refactoring my State object, so that the data I get from the Store is grouped under the same nested object I'd subscribe to, and then I'd at least have a single store.select.subscribe combo.

I wouldn't write this here if I didn't notice that you were using a .let method on a store object. What does it do? I couldn't google it out or find it in docs. I could look it up in the source, but I thought it would be easier to ask someone first and explain my problems with boilerplate. It's actually not the boilerplate (the typing part), but the reading part. Even I catch myself spending more time than it's needed to figure out what my code does in that place.

Thanks.