repeaterjs / repeater

The missing constructor for creating safe async iterators
https://repeater.js.org
MIT License
459 stars 12 forks source link

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

Closed brainkim closed 5 years ago

brainkim commented 5 years ago

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.