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

Auto-insert await for property accesses on Promise #31450

Closed
DanielRosenwasser opened this issue May 17, 2019 · 10 comments · Fixed by #32101
Closed

Auto-insert await for property accesses on Promise #31450

DanielRosenwasser opened this issue May 17, 2019 · 10 comments · Fixed by #32101
Assignees
Labels
Domain: Completion Lists The issue relates to showing completion lists in an editor Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented May 17, 2019

async function foo(x: Promise<string>) {
  x./**/
}

Go to the /**/ marker and request completions.

The proposal here is that all the completions for string should show up. When the user selects a string member, we should automatically await the value, so selecting toLowerCase changes the code to (await x).toLowerCase.

@fatcerberus
Copy link

This seems like a good idea but should only be done inside async functions if possible. In the example above, the function is not async so adding an await there would make the code invalid.

@DanielRosenwasser
Copy link
Member Author

Yes, thank you, that was one of the subtler points I forgot to add to this issue. It would be incredibly annoying/unsafe otherwise.

@DanielRosenwasser DanielRosenwasser added Domain: Completion Lists The issue relates to showing completion lists in an editor Suggestion An idea for TypeScript labels May 18, 2019
@fatcerberus
Copy link

Something else I just thought of: since this would be done via autocompletion, the feature probably shouldn't get any ideas about automatically converting the function to async - doing so is a breaking change in most cases since it forces the function into returning a promise.

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented May 22, 2019

Spoke with @littledan today, and once top-level await lands, we should consider doing this at the top level based on target and module options.

@DanielRosenwasser DanielRosenwasser added Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this labels May 25, 2019
@DanielRosenwasser DanielRosenwasser self-assigned this May 25, 2019
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.6.0 milestone May 25, 2019
@andrewbranch
Copy link
Member

Related: #30646

@andrewbranch
Copy link
Member

/cc @bterlson: Thoughts on this (and the work done already at #32101) as a partial solution for #30646? Personally, I’d still like to surface a quick fix and make sure we have a really good diagnostic message in addition to this, but I think completions are a good first line of defense for the last bullet point you mentioned:

  • The target of a member expression and T has the member (or maybe the property name is not in Promise).

@bterlson
Copy link
Member

I like it a lot, personally. Incrementally addressing the various bullets is fine by me, and I think this is probably one of the biggest rocks.

@DanielRosenwasser
Copy link
Member Author

The reason I put this proposal together was because when I spoke to @jonathandturner, I realized that better error messages (as in #30646) would only help TypeScript users, but would not help JavaScript users who aren't using type-checking.

@Kingwl
Copy link
Contributor

Kingwl commented Jun 27, 2019

Another quick fix is better, the current completion behavior seems cannot add async modifier simply as far as I know

@andrewbranch andrewbranch added Fix Available A PR has been opened for this issue and removed Help Wanted You can do this labels Jul 23, 2019
@xujif
Copy link

xujif commented Aug 13, 2019

a suggestion: add a new keyword to a function and translate to js with await all variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Completion Lists The issue relates to showing completion lists in an editor Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants