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

The way to fire an effect after effects are settled up #246

Closed
ValeryVS opened this issue Aug 8, 2017 · 10 comments
Closed

The way to fire an effect after effects are settled up #246

ValeryVS opened this issue Aug 8, 2017 · 10 comments

Comments

@ValeryVS
Copy link

ValeryVS commented Aug 8, 2017

I'm submitting a...


[x] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request

What is the current behavior?

There is no action, no function which fired when all effects are settled up. Where we can dispatch actions to fire other effects.

Expected behavior:

    this.onInit$ = this.actions$
      .ofType('@ngrx/effects/up')
      .switchMap((isAuthorized) => {
        return this.isAuthorized$;
      })
      .map((isAuthorized) => {
        if (isAuthorized) {
          this.store.dispatch({ type: '[posts] getAll' })
        }
      });

    @Effect()
    this.onGetPosts$ = this.actions$
      .ofType('[posts] getAll')
      .map(() => ... )

or, may be

  public ngrxOnRunEffects(resolvedEffects$: Observable<EffectNotification>): Observable<EffectNotification> {
    this.isAuthorized$
      .take(1)
      .subscribe({
        next: (isAuthorized) => {
          if (isAuthorized) {
            this.store.dispatch({ type: '[posts] getAll' })
          }
        },
      });
    return resolvedEffects$;
  }

Minimal reproduction of the problem with instructions:

There a few issues, of @ngrx/store/init action. Seems, that this action is not useful for described purpose anymore. So, we probably need another.
#103
#152

Version of affected browser(s),operating system(s), npm, node and ngrx:

ngrx v4

Other information:

Possible workaround:

dispatch action, somewhere after application start.
In the AppComponent for example.

this.store.dispatch({ type: '[app] init' });

And use it to dispatch other actions, that must be dispatched after all effects are settled up.

@wesselvdv
Copy link

wesselvdv commented Aug 9, 2017

Not sure why this would be needed. Have you tried the following operator:

 this.onInit$ = this.actions$
      .ofType(Actions.REESTABLISH)
      .startWith(new Actions.ReestablishAction()) // This triggers the specified action on load.
      .switchMap((isAuthorized) => {
        return this.isAuthorized$;
      })
      .map((isAuthorized) => {
        if (isAuthorized) {
          this.store.dispatch({ type: '[posts] getAll' })
        }
      });

@ValeryVS
Copy link
Author

ValeryVS commented Aug 9, 2017

@wesselvdv
What is Actions.REESTABLISH and Actions.ReestablishAction?
I don't see such variables in ngrx's Actions.

Also, I don't think, that I need to initialize first value in stream with startWith.
I need values from this.isAuthorized$, but this observable must wait until ngrx/effects are up.


I can write that in the service constructor

    this.isAuthorized$
      .take(1)
      .subscribe({
        next: (isAuthorized) => {
          if (isAuthorized) {
            this.store.dispatch({ type: '[posts] getAll' })
          }
        },
      });

[posts] getAll action fill be fired. But it must trigger an event, and no events are listening yet.

@wesselvdv
Copy link

wesselvdv commented Aug 9, 2017

@ValeryVS Those are placeholder action classes. You're using strings as actions, while the accepted way to define an action is using a class with a type parameter that contains a string.

Something like this from the example app:

export class LoginAction implements Action {
  readonly type = LOGIN;

  constructor(public payload: Authenticate) {}
}

Doing it like above gives you type checking on the payload. I just used a placeholder that follows that convention.

Anyhow, back on topic; where is isAuthorized coming from? Is that an initial state defined in a reducer? What is it?

@ValeryVS
Copy link
Author

ValeryVS commented Aug 9, 2017

@wesselvdv
Yes, initial state of isAuthorized is defined in a reducer.

Now I wrote this

    @Effect({ dispatch: false })
    this.onInit$ = this.actions$
      .ofType('[app] init')
      .switchMap(() => this.isAuthorized$)
      .map((isAuthorized) => {
        if (isAuthorized) {
          this.accountService.fetchAll();
          this.imgService.fetchAll();
          this.personService.fetchAll();
          this.roleService.fetchAll();
        }
      });

    public init() {
      this.store.dispatch({ type: '[app] init' });
    }

It works, but I need to laucn init() function in AppComponent. Through that I will know, when app is started and all ngrx effects are listen.

I suggest to make action, that will be dispatched automatically.
Before v4 it was init event #103 (comment), but now it isn't.

@wesselvdv
Copy link

wesselvdv commented Aug 9, 2017

That's way too complicated for this. The only thing you want is the initial state of a reducer, and you want to fire a boatload of actions based off it.

You can get the current state of a reducer using an operator:

@Effect()
    this.onInit$ = this.actions$
      .ofType('[app] init')
      .startWith('[app] init')
      .withLatestFrom(this.store.select(state => state.isAuthorized)) // < ---- Not sure where your prop is exactly.
      .map(([, isAuthorized]) => {
        if (isAuthorized) {
          this.accountService.fetchAll();
          this.imgService.fetchAll();
          this.personService.fetchAll();
          this.roleService.fetchAll();
        }
      });

Where you'll need to inject the store in your constructor as you would normally do to access a state from anywhere.

@MikeRyanDev
Copy link
Member

I will accept a PR that adds an "Root Effects Init" action that gets dispatched after the root effects have been started.

@timdeschryver
Copy link
Member

timdeschryver commented Aug 29, 2017

I have a working branch for this but I'm not 100% sure if its correct.
Is it OK if I submit a PR and we'll take it from there 😄 ?

EDIT: you can take a look at this branch - or just the commit.

@brandonroberts
Copy link
Member

@tdeschryver any movement on this?

@timdeschryver
Copy link
Member

@brandonroberts Didn't change anything since last time.
The problem is that I'm not 100% certain about the solution.
Is it OK to open a PR and take if from there?

@brandonroberts
Copy link
Member

@tdeschryver yes, that's fine.

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

No branches or pull requests

5 participants