Skip to content

Enhancement: [no-floating-promises] Provide a built-in SafePromise type #10666

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

Closed
4 tasks done
JoshuaKGoldberg opened this issue Jan 16, 2025 · 2 comments
Closed
4 tasks done
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@JoshuaKGoldberg
Copy link
Member

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/no-floating-promises

Description

Coming over from discussions such as #5844, #7008, #8404, reduxjs/redux-toolkit#4101, fastify/fastify#5498, #9869: one known user pain with @typescript-eslint/no-floating-promises is around frameworks/libraries that intentionally make "fire-and-forget" Promises. If a function's calls don't need an await by design, it's inconvenient to have the rule report on them by default.

The allowForKnownSafeCalls and allowForKnownSafePromises rule options make this better, but also require user linter configuration. For example, Redux Toolkit added a SafePromise type per allowForKnownSafePromises. Users can allowlist that type in their ESLint configs. This works but is inconvenient. Most users don't want to deeply hand-edit their ESLint configs for frameworks-specific nuances like this.

We are occasionally asked: can typescript-eslint provide its own SafePromise type that frameworks can use, and that no-floating-promises would allowlist/ignore by default?

Fail

declare function fromFramework(): Promise<void>;

await fromFramework();
// ❌ Lint report from no-floating-promises

Pass

import { SafePromise } from "@typescript-eslint/types";

declare function fromFramework(): SafePromise<void>;

await fromFramework();
// ✅ No report

Additional Info

I don't see an issue like this in our issue tracker, but know it's been suggested in at least some DMs to me. So I'm surfacing it here on behalf of users.

💖

@JoshuaKGoldberg JoshuaKGoldberg added enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jan 16, 2025
@JoshuaKGoldberg
Copy link
Member Author

JoshuaKGoldberg commented Jan 16, 2025

Now answering as a team member: I don't think this would be good, and here's a long explanation why...

We in typescript-eslint do not decide userland best practices or conventions. It would be inappropriate for us decide these things: we're not the universal experts of everything and are certainly not qualified to represent the vastness of the JS/TS world's various frameworks+libraries+tools. Rather, we attempt to represent the best practices and conventions that already exist in userland. We do our best to drop our personal biases -e.g. I abhor allowing void exemptions to no-floating-promises- and act fairly. We don't always succeed because we're human (and grossly underfunded as a team, hooray open source!), so our GitHub issues are always open for input. In other words, we try to interpret ecosystem-at-large needs and encode those into our rules: not the other way around.

The distinction is important because I don't want people to come away from these discussions thinking that we in typescript-eslint made a decision for the community, or that the aforementioned workarounds in frameworks are purely because of typescript-eslint. We just happen to be the only mainstream linter right now with a fully working no-floating-promises rule. The same issues happened in the past with TSLint and will happen in the future once Biome, deno lint, Oxlint, and others add typed linting as they all plan to.

All that is why we have not been in favor of adding a branded type like SafePromise to typescript-eslint itself. It would be inappropriate for us to tell "fire-and-forget" frameworks "we will report on your code by default unless you take a dependency on our package and use our type". typescript-eslint is not the conceptual source of linting for floating Promises: JS/TS + userland is. And even if we were willing to make that compromise, the same issue would pop up for every other typed linter implementing no-floating-promises. Linters would all have to either:

  • take a dependency on typescript-eslint - which is politically bizarre
  • provide their own SafePromise equivalent - which means frameworks would have to take an additional dependency for each linter
  • create some linter-agnostic SafePromise type - which is something nobody has initiated successfully
    • This could exist in a third-party package, which would be a heck of a lot of work to get adopted across the ecosystem
    • This could exist in TypeScript's types itself, but I would be surprised if they accepted a suggestion given how there's no actual runtime difference, and the "fire-and-forget" delineation isn't encoded into JavaScript

Putting it all together: IMO 👎 we shouldn't provide a SafePromise type ourselves. If one is separately established in the community, we could use that.

@JoshuaKGoldberg JoshuaKGoldberg added evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important and removed triage Waiting for team members to take a look labels Jan 28, 2025
@JoshuaKGoldberg
Copy link
Member Author

It's been >3 months since we added the evaluating community engagement label to this issue. In that time, we have not seen any engagement in favor of this feature request. We're still of the opinion that if something like this exists it cannot come from a single linter team like us -- it'd need to be centralized and/or from a community consensus.

Closing out as out of scope for this project. If anybody has significant new information for the issue of a built-in "SafePromise" type or similar, such as community consensus on a single type, please do post back in a new issue. We'd be happy to take a look.

For folks who want a no-floating-promises rule that respects a SafePromise type, you're welcome to use the docs in https://typescript-eslint.io/developers to build a rule/plugin that has that. You can always fork the rule from tseslint.

Cheers! 💖

@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Apr 21, 2025
@github-actions github-actions bot added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Apr 29, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

1 participant