-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
Update promise usage example for clarity #29746
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Thanks, I will review this in the coming days, because there are a few other issues I want to address. I will edit this PR directly. |
Preview URLs (comment last updated: 2023-11-23 07:08:23) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for the PR! For posterity, I made the following modifications:
- I added a very simple example of how you may stub the functions for experiments.
- I added correct counterparts for the incorrect examples.
- I added
async
/await
versions of the examples, something I've been wanting to do for a while. - I removed the "Common mistakes" section because it just restates the things already said in other places.
- I reordered the headings a bit to make it look more logical.
I have made substantial edits myself, but because I've already made this PR stall a while, I'll take liberty and merge it myself. If other reviewers find mistakes afterwards, I'll be happy to review a follow-on.
Description
Some edits to the promises examples to highlight the differences in the bad code example vs the good code example. PR addresses issue: #29743. Here's a summary of the changes
Motivation
Looking at the code examples for the promises, it is not exactly clear what makes a good promise call different from a bad promise call when considering the
return
statement. To avoid floating promises and race conditions, promises should always be returned either implicitly or explicitly, and this PR changes highlights that need. Changes will help the reader know that promises can have unexpected behavior if not properly returned.Additional details
N/A
Related issues and pull requests
Fixes: #29743
Fixes #29742
Fixes #28236