co-IT / ngrx-ducks

Improved Coding Experience for NgRx
https://co-it.gitbook.io/ngrx-ducks/
76 stars 10 forks source link

Extract NgRx ActionCreators #5

Closed GregOnNet closed 4 years ago

GregOnNet commented 5 years ago

Problem

If an action of certain state slice triggers an action of another slice you are forced to inject multiple Ducks in one Effect. This leads once again into service composition even if those services are stateless.

export class SomeEffect {
  load = createEffect(() => this.actions.pipe(
    whereType(this.firstDuck.load),
    // ...
    map((result) => this.secondDuck.doSomething.action(result)))
  );

  constructor(
    private actions: Actions,
    private dataService: SomeDataService,
    @Inject(FirstDuck) private firstDuck: Duck<FirstDuck>,
    @Inject(SecondDuck) private secondDuck: Duck<SecondDuck>,
    @Inject(ThirdDuck) private thirdDuck: Duck<ThirdDuck>,
    @Inject(FourthDuck) private fourthDuck: Duck<FourthDuck>,
    // ...
  ) {}
}

As the snippet shows this approach does not scale that well.

Proposal

To be as flexible as NgRx's built-in action creators a Duck could be destructured.

// first.duck.ts

@Ducksify({})
export class FirstDuck { /* ... */}

export const actions = actionsFrom(FirstDuck);

Benefit

GregOnNet commented 5 years ago

//cc Once again @skydever. Sorry, I know you are on holidays. :-D

GregOnNet commented 5 years ago

//cc @Markus-Ende

skydever commented 5 years ago

I am leaving next wednesday, still here 😉 but no worries about that ...

... somehow I like static methods for this, like I already do with reducer (eg. MyDuck.reducer at the module) - you quickly see where it is coming from, but maybe always exporting as reducer was just a bad choice concerning the name and static methods have a negative outcome in that case?

... how to trigger an action then at the effect, using the store.dispatch(MyDuck.setData({...})) method (and/or effect result action)?

... just thinking, if everything is static (possible I guess?), you end up injecting the store only 🤔 and a duck looks the same like it does now, and I really like how a duck looks like at the moment (also the latest additions) ... maybe a global store instance (window.store 😉 ) can make the injection go away completely and the dispatch() call looks the same MyDuck.setData(), MyDuck.loadAll.dispatch() just in upper case without this. ...

... again just writing before thinking it through, sorry for that ...

GregOnNet commented 5 years ago

Hi,

I am sure my description lacks some context. Destructure the Duck is not a replacement for the injection since this is one of the greatest things about it I believe. 🎉

This feature should add a better integration for Effects. In my experience you mostly need the action-creators to filter actions of the actions-Stream or to returning an action on the end of an effect. In this case the self-dispatching-action of a Duck is not needed. Injecting the whole Duck for just using it the action-creators may not be needed.

But no worries destructure would be just a new feature and does not introduce any breaking changes.

Markus-Ende commented 5 years ago

Sounds good to me! The only thing I'm not sure about is how this affects testability. With this approach it will not be possible to simply mock dependencies to other ducks. But on the other hand this should not be a problem, since I don't think it's necessary to mock them if we only use action creators.

skydever commented 5 years ago

I see, thx for the clarification :+1:

GregOnNet commented 5 years ago

Closing this for now since it would not be needed when #6 is implemented.

GregOnNet commented 5 years ago

6 has some downsides that's why I reopen this issue.

First tries have shown, that it is hard to extract an ActionCreator. Mapping the types somehow does not work as expected.

GregOnNet commented 5 years ago

Good news, the following type is the beginning of extracting ngrx compatible action creators form a Duck:

export type ActionCreatorCollection<T> = {
  [K in keyof T]: T[K] extends Function ? ActionCreator : never
};
GregOnNet commented 4 years ago

Closing this since it is covered in #9.