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

Feature: cancelNewWhenUncompleted #1477

Closed
PeterGursky opened this issue Dec 2, 2019 · 10 comments
Closed

Feature: cancelNewWhenUncompleted #1477

PeterGursky opened this issue Dec 2, 2019 · 10 comments

Comments

@PeterGursky
Copy link

PeterGursky commented Dec 2, 2019

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[x ] Feature request
[ ] Documentation issue or request
[ ] Support request => https://github.com/ngxs/store/blob/master/CONTRIBUTING.md
[ ] Other... Please describe:

What is the motivation / use case for changing the behavior?

Currently, the only ActionOption is cancelUncompleted. It would be convenient to have the opposite behavior, i.e. cancelling the new action, when the previous is not completed. CancelUncompleted works like mergeMap. This feature is the equivalent to exhaustMap.

Current behavior


@Action(Login, { cancelUncompleted: true })
  login(ctx: StateContext, { auth }: Login) {
    return this.userServerService.login(auth).pipe(   //<-- multiple logins on server
      tap(token => {
        ctx.setState({
          username: auth.name,
          token
        });
      })
    );
  }

Expected behavior


@Action(Login, { cancelNewWhenUncompleted: true })
  login(ctx: StateContext, { auth }: Login) {
    return this.userServerService.login(auth).pipe(
      tap(token => {
        ctx.setState({
          username: auth.name,
          token
        });
      })
    );
  }
@splincode
Copy link
Member

@markwhitfeld

@arturovt
Copy link
Member

arturovt commented Dec 5, 2019

@PeterGursky

CancelUncompleted works like mergeMap

This is falsy. It works like a switchMap. mergeMap produces parallel subscriptions wi/o cancelling previous ones.

I think this feature is unnecessary because it can be kept out of the core framework, if I needed to use an exhaustMap then I would do something like:

fromEvent(loginButton, 'click')
  .pipe(exhaustMap(() => this.store.dispatch(new Login({ username }))))
  .subscribe(() => {
    console.log('Successfully authenticated...');
  });

@splincode
Copy link
Member

@arturovt https://www.ngxs.io/v/master/recipes/component-events-from-ngxs
Please update documentation with your example

@arturovt
Copy link
Member

arturovt commented Dec 5, 2019

@splincode

This is too basic. If you want then go for it! 😉

@PeterGursky
Copy link
Author

PeterGursky commented Dec 5, 2019

Ok then. Thanks for the example. I wanted to prevent using @ViewChild() and fromEvent, because the behavior is going to change in Angular 9 (static property will be removed). But I can manage it with local state of the component.

@GustavoCostaW
Copy link

GustavoCostaW commented Feb 11, 2020

@arturovt @splincode

I think this feature is unnecessary because it can be kept out of the core framework, if I needed to use an exhaustMap then I would do something like:

I disagree, and a good reason for you guys consider this discussion is: I felt the same feeling as @PeterGursky and I've found this issue topic.

Let's create a scenario if you dispatch the same action from services AND multi-components? How do you manage it? you will need to create a singleton service to control the 'another action stream' and all those services/components will need to use that, I think it is a 'overengineer'.

With NgRx for example, you can control exactly the specific flattening strategy into the action stream observable where the async event happens.

e.g:

  @Effect()
  loadAnimal = this.actions$.pipe(
    ofType(saveAnimal),
    // here I can control my flattening strategy
    switchMap(([action]) => {
      return this.animals.save(action.payload).pipe(
        map(animal => new SaveAnimalSuccess(animal)),
        catchError(err =>
          of(
            new SaveAnimalError(
              'Failed to save the animal' 
            )
          )
        )
      );
    })
  );

Any new loadAnimal action dispatched will cancel the CURRENT HTTP call (I'm assuming save method returns an HTTP post obs$) as cancelUncompleted does, good!

but I'm able to change the flattening strategy to switchMap, mergeMap, concatMap, exhaustMap where the async action happens, for example, to exhaustMap, in the scenario below any new action dispatched is ignored until the first finishes.

    exhaustMap(([action]) => {
      return this.animals.save(action.payload).pipe(
        map(animal => new SaveAnimalSuccess(animal)),
        catchError(err =>
          of(
            new SaveAnimalError(
              'Failed to save the animal' 
            )
          )
        )
      );
    })

e.g: A login system and I don't want to make HTTP multiple calls, just the first one and I wanna ignore until the HTTP call has finished, or any async operation I wanna wait the first one has finished.

I can achieve the switchMap behavior with cancelUncompleted so, why not create a property for exhaustMap and have the same behavior? it will avoid creating another action stream to achieve it.

@oschlegel
Copy link

I have the same issue on my side. I have some data states which are requesting entities from a api. The actions to load those entities are dispatched from EVERY component which relies on a entity. My current solution to avoid duplicated actions / requests is to check a loading flag in the state model and cancel the action if the flag is already set.

I would really welcome a cancelNewWhenUncompleted or cancelDuplicated option. I would go one step further and provide a option like compareActions to compare and cancel actions not only based on their type but also by comparing their payload.

@markwhitfeld
Copy link
Member

For now this can be achieved using the Actions stream. in the exact same way as with ngrx.
I would be interested to add extensibility here as opposed to more custom options. The more custom options we add, the more the complexity of the API grows (the need for compareOptions above is a case in point).
I had been thinking about this 2 years ago, maybe it is worth looking into it again.
See issue #192 (Extensibility for Action Handlers)

@markwhitfeld markwhitfeld changed the title cancelNewWhenUncompleted Feature: cancelNewWhenUncompleted May 5, 2020
@splincode
Copy link
Member

@markwhitfeld what do you think move it issue to https://github.com/ngxs/store/discussions?

@arturovt
Copy link
Member

Thank you for submitting this feature request.
We are closing this issue due to the fact that:

  • This feature is not currently planned for the future
  • There has not been much support from other community members

You are welcome to re-open this issue with a further motivation for why this feature is essential for the core library or plugins.
If you would like to pursue the implementation of this feature we would encourage you to implement it in your own repository and share it with the community.

Thanks again

  • NGXS Core Team ❤️

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

6 participants