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

Add an aliveFlow that only calls .next() when the tree is still alive #1950

Closed
1 of 2 tasks
weiwei-lin opened this issue Aug 24, 2022 · 3 comments
Closed
1 of 2 tasks
Labels
enhancement Possible enhancement help/PR welcome Help/Pull request from contributors to fix the issue is welcome level: intermediate

Comments

@weiwei-lin
Copy link

Feature request

Add an aliveFlow that only calls .next() when the tree is still alive.
Also add something like the following to the async action doc.
CAVEAT: when running an async action, the tree might be destroyed before the promise is resolved, leading to errors.

Is your feature request related to a problem? Please describe.

This is very useful when running async actions on tree nodes that might be replaced.
For instance, I might have a SessionStore that fetch and update the session token automatically when it expires.
If the SessionStore is replaced (e.g. due to user switching to a different account), I'd love to abort any running async actions.

Note that this is different from #691 which requires cancelling the flow manually. #691 is less ergonomic when the flow is attached to a lifecycle hook.

Describe the solution you'd like

It can be implemented as

export function aliveFlow<R, Args extends any[]>(
    self: IAnyStateTreeNode,
    generator: Parameters<typeof flow<R, Args>>[0],
): ReturnType<typeof flow<R, Args>> {
  return flow(function* (...args) {
    const gen = generator(...args);
    let next = gen.next();
    while (!next.done) {
      const resolved = yield next.value;
      if (!isAlive(self)) {
        yield Promise.race([]);
      }
      next = gen.next(resolved);
    }
    return next.value;
  });
}

Are you willing to (attempt) a PR?

  • Yes
  • No
@coolsoftwaretyler
Copy link
Collaborator

Hey @weiwei-lin - I'm sorry it took so long for us to get back to you. Is this still an issue for you? Are you still interested in opening a PR? I totally understand if you've lost interest in this. Let me know what you think.

@coolsoftwaretyler coolsoftwaretyler added enhancement Possible enhancement help/PR welcome Help/Pull request from contributors to fix the issue is welcome level: intermediate labels Jun 25, 2023
@weiwei-lin
Copy link
Author

Hi. I attempted to work on a PR for this. However, the design I come up with (similar to the one I posted above) doesn't feel generic enough to be included the main package.

The main limitations are

  1. self has to be manually included in the params, and
  2. user can still run into the same issue when self is alive but the generator modifies a destroyed subbranch.

So I think it's better not to include aliveFlow in mobx-state-tree. Maybe we can have it in a package like mst-utils.

@coolsoftwaretyler
Copy link
Collaborator

Thanks, @weiwei-lin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Possible enhancement help/PR welcome Help/Pull request from contributors to fix the issue is welcome level: intermediate
Projects
None yet
Development

No branches or pull requests

2 participants