Change promises-guide references to Web IDL #1021
Conversation
|
Looks good so far. |
I commented inline on some possible wording here.
No need to stress this too much; I think just changing "fulfillment handler" to "fulfillment steps" is basically good enough. In particular for the "inline" usages (like "Return the result of transforming sourceCancelPromise with a fulfillment handler that returns undefined") I don't think we need to make that into a more complicated thing with sub-bullets and sub-algorithms.
This is good but could be done as an editorial followup, as "upon fulfillment" and "upon rejection" still exist and the semantics of using them separately is the same as using "reacting" together. So if it's going to delay merging don't worry about that. |
There was a problem hiding this comment.
All right, I've kept the changes to a minimum, just enough to unblock other spec changes. We can always do a more thorough refactor later on.
In principle, we should also keep the reference implementation in sync with the spec text changes. That means we should rename (and update?) the helpers in the reference implementation to align with Web IDL. But this can probably wait until another time as well.
| @@ -884,7 +884,7 @@ for="ReadableStreamAsyncIteratorPrototype">return( <var>value</var> )</h4> | |||
| 1. If *this*.[[preventCancel]] is *false*, then: | |||
| 1. Let _result_ be ! ReadableStreamReaderGenericCancel(_reader_, _value_). | |||
| 1. Perform ! ReadableStreamReaderGenericRelease(_reader_). | |||
| 1. Return the result of <a>reacting</a> to _result_ with a fulfillment handler that returns ! | |||
| 1. Return the result of <a>reacting</a> to _result_ with a fulfillment step that returns ! | |||
There was a problem hiding this comment.
I opted for a singular "fulfillment step" here, because I thought it sounded a bit weird with plural "steps":
Return the result of reacting to result with fulfillment steps that return ReadableStreamCreateReadResult(value, true, true)
In particular, I didn't like how the verb "return" must be plural in that sentence, while there's only a single step.
| 1. Return the result of <a>transforming</a> _flushPromise_ with: | ||
| 1. A fulfillment handler that performs the following steps: | ||
| 1. Return the result of <a>reacting</a> to _flushPromise_: | ||
| 1. If _flushPromise_ was fulfilled, then: |
There was a problem hiding this comment.
This was already very close to the new format from the example, so I went ahead and refactored it. I must say: I like this new format quite a bit! 😄
The spec text has broken links to the old promises guide. This is inconvenient for readers, but it also causes builds for spec PRs to fail (see my comment on #1011).
This PR fixes the links by pointing to their new Web IDL counterparts.
Todo list (non-exhaustive, feel free to suggest other changes):
Change pairs of "upon fulfillment / upon rejection" to a single "react to promise".(Not that important right now.)Fixes #1020
Preview | Diff