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

Redesign Repeater.prototype.return and the stop promise. #40

Closed
brainkim opened this issue Oct 13, 2019 · 0 comments
Closed

Redesign Repeater.prototype.return and the stop promise. #40

brainkim opened this issue Oct 13, 2019 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@brainkim
Copy link
Member

After some initial work on redesigning Repeater.prototype.throw to close #35, I think we should redesign Repeater.prototype.return and the stop promise as well, insofar as current behavior creates additional discrepancies between repeaters and async generators. Current behavior is problematic for the following reasons:

  1. The stop promise resolves to the value passed to Repeater.prototype.return.

This is behavior which we cannot replicated in an equivalent async generator, insofar as there is no way with try/finally blocks to inspect the original return value, only overwrite it. By exposing the value passed to return, we create opportunities for abstraction leaks where you have to treat calls to return on a repeater differently from calls to return on an async generator.

  1. Overwriting the return value of repeaters in the case of early return is probably a bad idea anyways.

Let’s say that we actually wanted this extra ability to inspect the value passed to return via stop. After all, with repeaters, you can race the stopping/returning of the repeater with another promise to ensure timely returns, which is something you cannot do within an async generator. However, even if we wanted to inspect the value passed to return with the stop promise, the only way it could be used would be to overwrite the final result of the iterator. All code which runs in the executor after the stop promise settles in the case of early return is equivalent to code within a finally block in an async generator, and established best practices dictate that we should not put any control flow statements in finally blocks. We already have no means of putting yield statements in a repeater’s “finally” block, insofar as push becomes a no-op function once the repeater is stopped, so it makes sense that we should also disallow the equivalent of return as well. Using the return value of the executor in the case of early return is like promoting the usage of return statements in finally blocks for async generators, which is something we don’t want to do.

Therefore, making the value passed to the return method inspectable becomes unnecessary. The executor’s return value should only be used in the case of a repeater’s normal completion. The only situation where we should overwrite the result of an early return is in the case of the executor throwing an error.

  1. Trying to figure out what to return from the executor is annoying.

In the implementation of various functions like the Repeater static method combinators, as well as the implementation of repeater utilities, I often wondered if we should return the stop promise from the executor to preserve the return behavior of async generators. By simply ignoring normal completions in the case of early returns, we get the default behavior of async generators for free, and we can stop worrying about returning the stop promise or using its value.

@brainkim brainkim added the enhancement New feature or request label Oct 13, 2019
@brainkim brainkim self-assigned this Oct 13, 2019
brainkim added a commit that referenced this issue Oct 15, 2019
Resolves #40
The stop promise no longer resolves to the value passed to return, and
executors are no longer able to change the final iteration in the case
of early return.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant