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

Issue with "Promise": Code used to demonstrate Chaining is misleading for new coders #2303

Closed
randycasburn opened this issue Feb 12, 2021 · 8 comments
Labels
Content:JS JavaScript docs

Comments

@randycasburn
Copy link
Contributor

MDN URL: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise

What information was incorrect, unhelpful, or incomplete?

The code sample for Chained Promises.

creates a new Promise on the fly and then uses direct call back functions for both resolve and reject handlers. That code sample was read by an new coder who completely misunderstood the intent and wrote this question on StackOverflow: https://stackoverflow.com/questions/66164973/promise-chain-does-not-wait-until-other-promise-is-resolved#66165074

A synopsis of the question's code follows:

function displayAll() {
    var myPromise = (new Promise(display1))
    .then((new Promise(display2))
    .then((new Promise(display3))
    .then(display4)));
}

And the demo code on MDN:

const myPromise =
  (new Promise(myExecutorFunc))
  .then(handleFulfilledA,handleRejectedA)
  .then(handleFulfilledB,handleRejectedB)
  .then(handleFulfilledC,handleRejectedC);

Specific section or headline?

Chained Promises

What did you expect to see?

To better demonstrate how Chained promises are used in the wild, it would be more productive/instructive to separate all the move parts. Something along the lines of the Promise Constructor sample code as a starting point.

const promise1 = new Promise((resolve, reject) => {
  setTimeout(() => {
    resolve('foo');
  }, 300);
});

Followed by the chaining thus:

promise1
   .then((value) => { return value + ' and bar'; })
   .then((value) => { return value + ' and bar again'; })
   .then((value) => { return value + ' and again'; })
   .then((value) => { return value + ' and again'; })
   .then((value) => { console.log(value) });

While this code is more verbose than it needs to be, it is clear a function is required as the parameter(s).

Did you test this? If so, how?

The StackOverflow question indicates this causes some confusion.

MDN Content page report details
@CreaTorAlexander
Copy link
Contributor

CreaTorAlexander commented Feb 12, 2021

I can update this 👍

@CreaTorAlexander
Copy link
Contributor

@randycasburn I agree that it is hard to understand in the current example but what I think what is good in it is to show the purpose with handleFullfiled and handleReject this would get lost when I update it with your suggestion.
Maybe there is a way to make clear that you shall not pass it to the then's arguments list and also to show the promise behavior with the fullfilled or rejected function.

@randycasburn
Copy link
Contributor Author

randycasburn commented Feb 12, 2021 via email

@randycasburn
Copy link
Contributor Author

So I completely agree with you. My suggestion was not meant to be complete. But to correct that, let me offer a more complete version to address your valid concern:

Chained Promises

The methods promise.then(), promise.catch(), and promise.finally() are used to associate further action with a promise that becomes settled.

The .then method takes up to two arguments. These are callback functions for the resolved and rejected cases of the Promise. In the examples below handleResolve and handleRejected represent these two callback functions. These methods also return a newly generated promise object, which can optionally be used for chaining; for example:

const myPromise = new Promise((resolve, reject) => {
  setTimeout(() => {
    resolve('foo');
  }, 300);
});

myPromise
  .then(handleResolvedA, handleRejectedA)
  .then(handleResolvedB, handleRejectedB)
  .then(handleResolvedC, handleRejectedC);

Processing continues to the next link of the chain even when a .then() lacks a callback function that returns a Promise object. Therefore, a chain can safely omit every rejection callback function until the final .catch().

Handling a rejected promise in each .then has consequences further down the promise chain. Sometimes there is no choice because an error must be handled immediately. In such cases we must throw an error of some type to maintain error state down the chain. On the other hand, in the absence of an immediate need, it is simpler to leave out error handling until a final .catch() statement. A .catch() is really just a .then() without a slot for a resolve callback function.

myPromise
  .then(handleResolvedA)
  .then(handleResolvedB)
  .then(handleResolvedC)
  .catch(handleRejectedAny);

An implementation of these callbacks might look something like this in actual code using arrow function expressions:

promise1
   .then( value => { return value + ' and bar'; })
   .then( value => { return value + ' and bar again'; })
   .then( value => { return value + ' and again'; })
   .then( value => { return value + ' and again'; })
   .then( value => { console.log(value) })
   .catch( err => { console.log(err) });

The termination condition of a promise determines the "settled" state of the next promise in the chain. A "resolved" state indicates a successful completion of the Promise while a "rejected" state indicates a lack of success. The return value of each resolved promise in the chain is passed along to the next .then() while the reason for rejection is passed along to the next rejection handler function in the chain.

The promises of a chain are nested like Russian dolls, ... // editor's note: continue with existing content as written


The rest of the content remains as written

Note the removal of this code block:

handleFulfilled(value)       { /*...*/; return nextValue;  }
handleRejection(reason)  { /*...*/; throw  nextReason; }
handleRejection(reason)  { /*...*/; return nextValue;  }

Please let me know if you have any questions.

@CreaTorAlexander
Copy link
Contributor

CreaTorAlexander commented Feb 13, 2021

Sounds good :) Would you like to do the PR?

@randycasburn
Copy link
Contributor Author

randycasburn commented Feb 14, 2021 via email

@randycasburn
Copy link
Contributor Author

PR created.

@CreaTorAlexander CreaTorAlexander removed their assignment Feb 16, 2021
@github-actions github-actions bot added the idle label Dec 8, 2021
@wbamberg
Copy link
Collaborator

Looks like this is closed by #3196.

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

No branches or pull requests

4 participants