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

Non-nullable promises shouldn't be allowable in conditions #34717

Closed
4 of 5 tasks
twavv opened this issue Oct 24, 2019 · 3 comments
Closed
4 of 5 tasks

Non-nullable promises shouldn't be allowable in conditions #34717

twavv opened this issue Oct 24, 2019 · 3 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@twavv
Copy link

twavv commented Oct 24, 2019

Search Terms

Promise, conditions, if, truthiness, coercsion, known-true

Related

#9041, #33178

Suggestion

Non-nullable promises should not be implicitly converted to truthy inside of conditions.

Use Cases

This helps prevent common bugs where promises aren't await-ed (see below).

Examples

async isAllowedToDoTheSuperSecretThing(user: User) {
    /* ... */
}

async doTheSuperSecretThing(ctx: RequestContext) {
  if (isAllowedToDoTheSuperSecretThing(ctx.user)) {
    /* do the super secret thing */
  }
}

should result in an error since isAllowedToDoTheSuperSecretThing returns a non-nullable promise and is never falsey.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

This definitely falls into the camp of goal one:
Statically identify constructs that are likely to be errors.

This would be breaking but so is #33178.

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Oct 29, 2019
@twavv
Copy link
Author

twavv commented Nov 26, 2019

@RyanCavanaugh This was tagged "Awaiting More Feedback." Does that have any specific meaning or does it just generically mean "this is on the back-burner"?

@twavv
Copy link
Author

twavv commented Dec 13, 2019

This is available as a plugin for ESLint: typescript-eslint/typescript-eslint#612.

The downside is that it requires type checking in ESLint which is pretty slow since it generally requires the whole codebase to be checked (whereas when just developing with the language server running, it's pretty fast).

@andrewbranch
Copy link
Member

Implemented in #39175

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants