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

Consistently use async/await rather than then. #70

Closed
mpiroc opened this issue Sep 24, 2018 · 5 comments
Closed

Consistently use async/await rather than then. #70

mpiroc opened this issue Sep 24, 2018 · 5 comments
Assignees

Comments

@mpiroc
Copy link
Contributor

mpiroc commented Sep 24, 2018

In some places, we still use then where async/await would be more clear. We shouldn't have "legacy" code at this early stage of development. We should give the existing codebase a once-over to make sure that we're using async/await.

@stevejroberts
Copy link
Contributor

Not necessarily disagreeing but I'm not clear why 'then' (or any task/promise continuation syntax) is considered 'legacy' or not as clear?

Is there a typescript style guide documented somewhere we should follow and discuss to come up with the right set of tslint settings?

@mpiroc
Copy link
Contributor Author

mpiroc commented Sep 26, 2018

async/await is preferred because it allows you to write flat code, rather than adding a level of nesting for each asynchronous call. This makes asynchronous code much easier to read, write, and reason about. The awkward nesting of Tasks/Promises/etc. is the motivation for the async/await feature in the first place.

I don't see await vs then specifically called out in any popular style guide, but any async/await guide will explain the advantages. Some examples:

I don't think that a tslint rule is appropriate, because there are still cases where you might want then:

const promises = []
for (const i = 0; i < 10; i++) {
    promises.append(makePromise(i).then(...))
}

await Promise.all(promises)

You could avoid then by extracting makePromise(i).then(...) into a helper function, but using then here is simpler.

This issue is more about cases like this:

foo(): Promise<string> {
    return somePromise.then(result => {
        return `Hello, ${result}!`
    })
}

Compare to:

async foo(): Promise<string> {
    const result = await somePromise
    return `Hello, ${result}!`
}

@awschristou
Copy link
Contributor

I have a preference towards async/await as it seems to cut down on explicit Promise usage. In one case, I had wrapped a function in a Promise constructor, which accidentally ate thrown Errors. I chalk that up to gaining familiarity with the language though too.

VS Code Extension development supports promises broadly - https://code.visualstudio.com/docs/extensionAPI/patterns-and-principles#_promises

@stevejroberts
Copy link
Contributor

stevejroberts commented Sep 27, 2018

As I said, I don't necessarily disagree. I checked with our Javascript SDK team regarding your use of the term 'legacy' for 'then' usage and they don't consider it such and in fact raised the same discussion that async/await is preferred but sometimes 'then' is useful. Therefore I don't think a broad 'must not use then()' mandate is appropriate.

Our style guide, when we have one, should therefore likely state 'prefer async/await over then()' and show the examples you list as the reason why. Perhaps this issue should mutate into one that creates said style guide in the repo.

@mpiroc
Copy link
Contributor Author

mpiroc commented Sep 27, 2018

@steveataws Thanks, that sounds good. I worded this issue poorly--I agree that "prefer" is the right terminology.

@mpiroc mpiroc closed this as completed Oct 11, 2018
awschristou pushed a commit that referenced this issue Mar 30, 2020
Authored-by: Alan Bogusiewicz <alabogus@amazon.com>
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