Hi all,
I’d like to formalize our writing style for JavaScript/Typescript asynchronous code.
The guidelines proposed below would apply to all modules, be required for newly introduced code, and encouraged for refactored code.
Proposal
P1. async/await is the default way to write asynchronous code. Concretely: no .then() / .catch() / .finally() chains, errors are handled with try/catch. Note that jQuery promises are compatible with async/await (i.e., no .done() / .fail() / $.Deferred()).
P2. Three exceptions.
new Promise(...)is allowed only to wrap an API that is based on callbacks (events, timers,require([...], callback)).Promise.all,Promise.allSettledandPromise.raceare of course encouraged to coordinated promises, as their semantic cannot be expressed with async/await alone.- Deliberate fire-and-forget: calling an
asyncfunction without waiting for it, with a terminal.catch()so that the failure is reported. A comment is expected in this case
Actions
If the rules proposed above are accepted, I propose to do the following actions.
A1. Enforce it with ESLint where we can. Our shared ESLint configuration already loads eslint-plugin-promise. I propose to enable promise/prefer-await-to-then as an error, with its default options, and to convert the existing occurrences in the same change. Is it expected to be a small change, but I allow myself to add eslint ignore comments if there is a risk of breaking things.
See below an example of how to handle a fire-and-forget call (P2.3)
// eslint-disable-next-line promise/prefer-await-to-then Fire-and-forget: the UI must not wait for this.
refreshCounters().catch(reportError);
I don’t plan to add a linting rule for older JavaScript not covered by ESLint.
A2. Document it. I propose to add this rule to the JavaScript Best Practices.
Motivation
Error handling is considered harder to get wrong MDN lists this among the common mistakes (“It is important to always return promises from then callbacks”). In the following example, if response.json() or update() fails, the user never sees the error:
function save(data) {
showProgress();
fetch(url, { method: "PUT", body: JSON.stringify(data) })
.then((response) => {
if (response.ok) {
response.json().then((saved) => {
update(saved);
showSuccess();
});
} else {
showError(response.statusText);
}
})
.catch((error) => showError(error.message));
}
The same code with await has no way to leave that step out of the error handling:
async function save(data) {
showProgress();
try {
const response = await fetch(url, { method: "PUT", body: JSON.stringify(data) });
if (!response.ok) {
showError(response.statusText);
return;
}
update(await response.json());
showSuccess();
} catch (error) {
showError(error.message);
}
}
It is easier to debug. In Chrome, Edge and Node, the stack trace of an error includes the calling async functions only when they use await; the V8 team states that the mechanism “won’t help with Promise only code”. Browser debuggers also step over an await line by line (Chrome, Firefox), where a chain requires a breakpoint in each callback.
The V8 team’s advise it: “favor async functions and await over hand-written promise code”, and they report that it is at least as fast.
Conclusion
Finally, I want to acknowledge a few common traps with async/await:
- Awaiting several independent operations one after the other when they could run concurrently. This is why P2.2 exists (i.e.,
Promise.all). - Forgetting an
await. ESLint can detect it with@typescript-eslint/no-floating-promises, which needs type information. This is what Clément proposed in Enabling recommended type-checking rules for ESLint + TypeScript; the two proposals complement each other, and this one does not depend on it. Most IDEs already raise a warning in this case
Here is my +1 for P1, P2, A1, and A2.
Thanks