WIP: refactor(Effects): Remove runEffects and StateUpdates, add NgModule

[MikeRyanDev]

AKA the release where I delete most of this library.

In @ngrx/effects v1, providers were created dynamically via runEffects(...effects: Type | Type[]). However, the providers generated this way were not statically analyzable and so they would not work with the offline compiler.

I'm looking for comments on the new, offline compiler friendly API:

import { Injectable, OnDestroy } from '@angular/core';
import { Subscription } from 'rxjs/Subscription';
import { Store } from '@ngrx/store';
import { Effect, mergeEffects, Actions } from '@ngrx/effects';


@Injectable()
export class AuthEffects implements OnDestroy {
  subscription: Subscription;

  constructor(private actions: Actions, store: Store<T>) {
    this.subscription = mergeEffects(this).subscribe(store);
  }

  @Effect() login$ = this.actions$
    .ofType('login')
    .mergeMap(...)

  ngOnDestroy() {
    this.subscription.unsubscribe();
  }
}

Note: services in RC5 are automatically instantiated if they implement the OnDestroy interface. This has the added benefit of automatically starting / stopping effects dynamically, like during routing.

This has been published with the @beta tag. Get it with npm install @ngrx/effects@beta

Also in this release is the new release structure outlined in ngrx/core#4

cc @fxck @btroncone @robwormald

[btroncone]

LGTM 👍
You know I was fond of previous API but it's time has come. 😄

[fxck]

Yeah, looks good to me. What I would like to see in README:

• where has update$.state gone to and how to get it back when you need it
• whether ngOnDestroy is required and how to instantiate the effect without it(when you want your effects to run always and everywhere)

as a side note:

• since ofType presumably accepts multiple actions, how about naming it ofTypes instead
• while you are at it, can we get that end, terminate, finish operator? I know there's https://github.com/ReactiveX/rxjs/blob/master/src/operator/ignoreElements.ts but that name, ugh.

[awerlang]

I think this is good. The previous implementation was a bit too much.

You know I was fond of previous API but it's time has come. 😄

It should be easy enough for users to reproduce the previous experience:

@Injectable()
export class BaseEffects implements OnDestroy {
  subscription: Subscription;

  constructor(private actions: Actions, store: Store<T>) {
    this.subscription = mergeEffects(this).subscribe(store);
  }

  ngOnDestroy() {
    this.subscription.unsubscribe();
  }
}

@Injectable()
export class AuthEffects extends BaseEffects {

  @Effect() login$ = this.actions$
    .ofType('login')
    .mergeMap(...)

}

The downside with this is when we need to inject more providers in the constructor.

What about creating an OnSubscribe interface and letting effect classes implementing it instead of initializing in constructors:

export interface OnSubscribe<T>() {
  onSubscribe(actions: Actions, store: Store<T>): void;
}

@Injectable()
export class BaseEffects implements OnSubscribe, OnDestroy {
  actions$: Actions;
  subscription: Subscription;

  onSubscribe(actions: Actions, store: Store<T>): void {
    this.actions$ = actions; // or inject in constructor
    this.subscription = mergeEffects(this).subscribe(store);
  }

  ngOnDestroy() {
    this.subscription.unsubscribe();
  }
}

@Injectable()
export class AuthEffects extends BaseEffects {

  @Effect() login$ = this.actions$
    .ofType('login')
    .mergeMap(...)

}

Yet another option to not rely on constructor in base class for acquiring the action$ source, is making effects, functions:

  @Effect() login$ = actions$ => actions$
    .ofType('login')
    .mergeMap(...)

WDYT?

[awerlang]

while you are at it, can we get that end, terminate, finish operator?

I'd prefer configuring the decorator:

@Effect(/* dispatch: boolean = true */ false) login$ = actions$ => actions$
    .ofType('login')
    .do(...)

[fxck]

☝️ that looks reasonable.

[MikeRyanDev]

where has update$.state gone to and how to get it back when you need it

Recreating the updates stream is pretty straightforward and makes it more clear how to apply selectors in effects:

actions$
  .ofType('login')
  .withLatestFrom(store$)
  .map(([ action, state ]) => ({ action, state }))

whether ngOnDestroy is required and how to instantiate the effect without it(when you want your effects to run always and everywhere)

Yes, as of today if you want your service to be automatically instantiated it must implement the OnDestroy interface.

What about creating an OnSubscribe interface and letting effect classes implementing it instead of initializing in constructors:

I don't think this is technically possible yet.

I'd prefer configuring the decorator:

Yeah, configuring the decorator like that seems like a reasonable improvement of the API. I think I'd prefer it this way to leave room for future configuration options:

@Effect({ dispatch: false }) login$ = this.actions$

since ofType presumably accepts multiple actions, how about naming it ofTypes instead

I'm following what they are doing in redux-observable. See redux-observable/redux-observable#95

[fxck]

Recreating the updates stream is pretty straightforward and makes it more clear how to apply selectors in effects:

yes, and it's gonna be a source of confusion nevertheless, that's why it better be mentioned in readme :)

[meenie]

Doesn't seem to be needed.

[MikeRyanDev]

I haven't fully implemented the typings for the lift method and importing the Operator interface will be required to do so. See: https://github.com/ngrx/store/blob/master/src/store.ts#L25

[meenie]

I've tried @ngrx/effects@beta in my app and it works great! Only minimal changes to get it to work. I'm even getting access to the store and pulling out a value with something like:

  @Effect() redirectAfterLogin$ = this.actions$
    .ofType(AuthActions.REDIRECT_AFTER_LOGIN)
    .withLatestFrom(this.store.let(appSelectors.getAuthRedirectUrl()))
    .do(([action, url]) => router.navigateByUrl.next(url))
    .mapTo(this.authActions.resetRedirectAfterLogin())

withLatestFrom() is amazing :)

Great work!

[meenie]

Due to the new version of Effects, I tried to see if I could inject the Router into my Effects Class and I'm getting this error: zone.js:461Unhandled Promise rejection: Bootstrap at least one component before injecting Router.. So I'm still having to use the RouterPatch hack.

Any thoughts on this?

[brandonroberts]

@meenie can you make a plunker with this issue?

[MikeRyanDev]

Problems with this proposed API:

• Not currently possible to inject services that require the application to have been bootstrapped
• Creating the effects subscription in the constructor makes the effects service harder to test
• Requiring developers to implement the OnDestroy interface is confusing

I'll try to address these issues with a new API sometime early this week.

[btroncone]

@Effect({ dispatch: false }) login$ = this.actions$

I really like this approach ☝️ 👍

[stefanoslig]

Is there any way to unit test an effect? I want to find a way to send an action to my effect. In the current version there is the sendAction function.