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

Confusing use of transforming with and Upon fulfillment. #621

Closed
jyasskin opened this issue Feb 12, 2015 · 4 comments
Closed

Confusing use of transforming with and Upon fulfillment. #621

jyasskin opened this issue Feb 12, 2015 · 4 comments

Comments

@jyasskin
Copy link
Member

Several algorithms contain steps like the following two:

  1. Let p be transforming responseArrayPromise with onFulfilled.
  2. Upon fulfillment of p with value responseArray, perform the following substeps, onFulfilled, in parallel:

onFulfilled isn't defined except in the Upon fulfillment substeps, but it's odd to see it used twice. In some cases, like add(), the second use is inconsistent because it tries to resolve the already-fulfilled promise. Some other cases, like addAll(), define onFulfilled twice, in different ways.

I suspect you don't mean to use "Upon fulfillment" after "transforming" at all.

@jyasskin
Copy link
Member Author

@domenic to check our understanding of the shorthand phrases.

@jungkees
Copy link
Collaborator

Thanks for spotting it, @jyasskin. Some part of the shorthand expressions were redundant as you pointed. (The example in the original comment was not redundantly phrased but should have been simplified.) Also, I removed the name of the handler, onFulfilled, as it's not needed.

@domenic review would be appreciated: 5aa0ca2. I believe returning values from within a fulfillment handler to resolve the transformed promise is a right usage, right? I've done that a lot referring to the example 2 in http://www.w3.org/2001/tag/doc/promises-guide#shorthand-reacting.

@domenic
Copy link
Contributor

domenic commented Feb 13, 2015

5aa0ca2 LGTM. And yeah, returning or throwing in the handler is perfect.

@jungkees
Copy link
Collaborator

Closing. Other incorrect usages have been addressed in #627 and #628.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants