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

Propagate resetState to canceled child tasks #461

Open
jelhan opened this issue Apr 2, 2022 · 3 comments
Open

Propagate resetState to canceled child tasks #461

jelhan opened this issue Apr 2, 2022 · 3 comments
Labels
feature-request Request for feature v2 Applies to ember-concurrency v2

Comments

@jelhan
Copy link

jelhan commented Apr 2, 2022

It seems that there isn't an option to reset the state of child tasks when canceling a parent task.

Assuming I have a parent and child task like this:

class Foo {
  @task
  *parent() {
    yield this.child.perform();
    return 'foo';
  }

  @task
  *child() {
    return 'bar';
  }
}

I can reset the state of the parent task when canceling it by passing { resetState: true } option:

cancel() {
  this.parent.cancelAll({ resetState: true });
}

But that option does not seem to propagate to cancellation of the child task.

As a work-a-round I need to cancel the child task manually and pass the { resetState: true } option to it as well:

cancel() {
  this.child.cancelAll({ resetState: true });
  this.parent.cancelAll({ resetState: true });
}

It would be great if I the resetState option propagates to all child tasks instead. Maybe controlled by another option for cancelAll.

@maxfierke maxfierke added feature-request Request for feature v2 Applies to ember-concurrency v2 labels Apr 2, 2022
@maxfierke
Copy link
Collaborator

This is a great idea! I could see cases where you might not want this behavior, but it makes sense that the default should probably be to reset child task state too.

@machty
Copy link
Owner

machty commented Apr 7, 2022

class Foo {
  @task
  *parent() {
    yield this.child1.perform();
    yield this.child2.perform();
    yield this.child3.perform();
    return 'foo';
  }
  // ...
}

Let's say parent is cancelled while awaiting child2; is the idea that child1 (in additional to child2) would have its state reset because it was previously performed by child1?

I can imagine some trickiness in the mental model and potentially how we implement this if, say, other tasks might be calling child1 directly; and we might not be able to say this is a backwards-compatible change.

@maxfierke
Copy link
Collaborator

I can imagine some trickiness in the mental model and potentially how we implement this if, say, other tasks might be calling child1 directly; and we might not be able to say this is a backwards-compatible change.

agreed. it's definitely a change that would have to come under a toggle of some sort or next major release. would also add that it might make sense to have it entangled w/ linked() vs unlinked() on it too (i.e. unlinked() shouldn't trigger that behavior), but also there should be an opt-out at the cancelAll level too (e.g. cancelAll({ resetState: true, skipChildren: true }) or something

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for feature v2 Applies to ember-concurrency v2
Projects
None yet
Development

No branches or pull requests

3 participants