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

RFC: "Action class" and "Reducer factory" to define them easier #1345

Closed
stm32p103 opened this issue Sep 28, 2018 · 4 comments
Closed

RFC: "Action class" and "Reducer factory" to define them easier #1345

stm32p103 opened this issue Sep 28, 2018 · 4 comments

Comments

@stm32p103
Copy link

stm32p103 commented Sep 28, 2018

Please excuse me if this RFC is duplicate since some people would have proposed something like "automatically define actions" or "easy way to define actions".

I want to propose the way to define actions and reducers easier. Example is here.

Action class to define actions

Instead of writing action name constants, define class with static method decorated with ToAction . ToAction decorator wraps original method and returns action which type is "method.class" and payload of its original return value.

For example, Counter example can be written as below.

export class CounterActions {
    @ToAction() static increment(): any {} 
    @ToAction() static decrement(): any {} 
    @ToAction() static preset( n: number ): any { return n; }
}

Reducer factory to define reducer

Instanciate ReducerFactory<T> and call add( Class.StaticMethod, reducer ) to add reducers for respective actions, then create() to create reducer.

Note that there is no need to write action name constants. And auto-complete will help you find actions you need.

const factory = new ReducerFactory<number>();

// add( action, reducer: ( state: T, payload? any ) => T );
factory.add( CounterActions.increment, ( count: number ) => count + 1 );
factory.add( CounterActions.decrement, ( count: number ) => count - 1 );
factory.add( CounterActions.preset, ( count: number, preset: number ) => preset );

// Define reducer
export const counterReducer = factory.create( 0 );  // initial value = 0

Dispatch actions

@Component({
  selector: 'app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.css']
})
export class AppComponent {
    count$: Observable<number>;
    constructor( private store: Store<any> ) {
        this.count$ = this.store.pipe( select('count') );
    }
    increment() {
        this.store.dispatch( CounterActions.increment() );
    }
    decrement() {
        this.store.dispatch( CounterActions.decrement() );
    }
    reset() {
        this.store.dispatch( CounterActions.preset( 0 ) );
    }
}

Define effects

Instead of ofType operator, use payloadOf operator and get payload directly.

@Injectable()
export class EffectTest {
    constructor( private actions$: Actions ) {}

    @Effect( { dispatch: false } ) preset$ = this.actions$.pipe(
        payloadOf( CounterActions.preset ),
        map( payload => console.log( JSON.stringify( payload ) ) )
    );
}

If accepted, I would be willing to submit a PR for this feature

[ x ] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

@timdeschryver
Copy link
Member

@stm32p103 thanks for the proposal! If this pattern works for you that's great, but I think we won't be adding this to the core API.

Because the reasons I'm thinking:

  • this doesn't work well with "good action hygiene"
  • this adds some "magic" to the code; making it harder to understand for users
  • I'm not how typesafe this can be

@stm32p103
Copy link
Author

@timdeschryver thank you for your comment.

After read your comment and checked the video "Good Action Hygiene with NgRx Mike Ryan", I revised my proposal more "good action hygiene" friendly, less magical and more typesafe (maybe).

If there is any chance, please re-review my revised proposal. Thank you.


In revised proposal, Action definition can be written as shown below.

export class CounterActions {
    static increment = ToAction( '[Counter] Increment', () => {} );
    static decrement = ToAction( '[Counter] Decrement', () => {} );
    static preset    = ToAction( '[Counter] Preset',    ( n: number ) => n );
}

As for reducer definition, it can be written as shown below. And now type inference is available.

const factory = new ReducerFactory<number>();

factory.add( CounterActions.increment, ( count ) => count + 1 );
factory.add( CounterActions.decrement, ( count ) => count - 1 );
factory.add( CounterActions.preset,    ( count, preset ) => preset );

// create( initial value );
export const counterReducer = factory.create( 0 );

Typescript infers type of payload from specified actions (eg: CounterActions.preset -> number).
Type inference

When arguments of reducer are wrong:
Type inference error

When actions are incompatible:
Type inference error

(However "Action constant" dictionaly populated in "ngrx-utility", it is little magical though.)

@stm32p103
Copy link
Author

@timdeschryver Sorry, after I posted the comment above, I realized that I misunderstood what "typesafe" means. So I take back the revised proposal.

@timdeschryver
Copy link
Member

👍 Thanks for taking the time to write a comprehensive and clear response.
Like I said before, if this works for you, you can start using it (or making it a library) but I don't think this will become part of the ngrx core api.

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

No branches or pull requests

2 participants