Skip to content

Bug: [no-floating-promises] allowForKnownSafeCalls only matches types, not values #10686

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 19, 2025 · 7 comments
Closed
4 tasks done
Labels
bug Something isn't working locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. triage Waiting for team members to take a look

Comments

@JoshuaKGoldberg
Copy link
Member

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Issue Description

Given this setting for @typescript-eslint/no-floating-promises:

"@typescript-eslint/no-floating-promises": ["error", {
    "allowForKnownSafeCalls": [
        { "from": "package", "name": "useNavigate", "package": "react-router" }
    ]
}]

...I would have thought this code would not produce a lint report:

import { useNavigate } from 'react-router';

const navigate = useNavigate();
navigate('/new/page');

...but it does on my machine:

/Users/josh/repos/repros/index.ts
  4:1  error  Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator  @typescript-eslint/no-floating-promises

Sparked by a #help question in Discord: https://discord.com/channels/1026804805894672454/1326739019387899974

Reproduction Repository Link

https://github.com/JoshuaKGoldberg/repros/tree/repro-tseslint-no-floating-promises-react-router-allow-useNavigate

Repro Steps

  1. clone the repo at that branch
  2. npm install
  3. npm run lint

Versions

package version
@typescript-eslint/eslint-plugin 8.20.0
TypeScript 5.7.3
ESLint 9.18.0
@JoshuaKGoldberg JoshuaKGoldberg added bug Something isn't working triage Waiting for team members to take a look labels Jan 19, 2025
@43081j
Copy link
Contributor

43081j commented Jan 19, 2025

This is because useNavigate isn't the type, it is the function being called. allowForKnownSafeCalls operates on the type, and in this case, that is NavigateFunction (since useNavigate(): NavigateFunction)

if you set "name": "NavigateFunction", it will work

maybe that's just a docs thing or maybe the rule should operate on symbol names instead

@JoshuaKGoldberg JoshuaKGoldberg changed the title Bug: [no-floating-promises] allowForKnownSafeCalls isn't allowing react-router's useNavigate Bug: [no-floating-promises] allowForKnownSafeCalls only matches types, not values Jan 19, 2025
@JoshuaKGoldberg
Copy link
Member Author

JoshuaKGoldberg commented Jan 19, 2025

Aha, thank you! I think this is what @bradzacher mentioned ... somewhere, I forget - that the selector only works on types, not values, at the moment. Edit: no, I misread - this is working as intended.

Good question on whether this is a bug or a docs issue. IMO this is a bug because we want to let users specify allowed known safe functions. The specifier is even called "type or value specifier". cc @typescript-eslint/triage-team

@bradzacher
Copy link
Member

bradzacher commented Jan 19, 2025

The issue isn't just that though.

The promise doesn't come from the useNavigation function. It comes from the navigate function. That function is locally defined so it will never match the selector you've configured.

Put another way - your config should prevent flagging on

useNavigation();

But not on

const cb = useNavigation();
cb(); // error here

@43081j
Copy link
Contributor

43081j commented Jan 19, 2025

Though still this option operates on types right now and useNavigation isn't that

That is possibly still an issue if we want the rule option to also allow allowing of symbol names rather than only types

@kirkwaiblinger
Copy link
Member

FYI @JoshuaKGoldberg #10660 also/already tracks matching values with TypeOrValueSpecifier. Seemingly, though, since that won't solve this issue, what remains on this issue is more of a "what should be done about patterns that can't easily be matched by a type/value selector?"

@bradzacher
Copy link
Member

what should be done about patterns that can't easily be matched by a type/value selector?

Ideally the libraries return some nominally typed promise that a user can match the rule on - which was what we tried to push people to.

Alternately PromiseLike is currently ignored by the rule as per other issues because of its existing usages in such APIs.

@JoshuaKGoldberg
Copy link
Member Author

Yeah I don't know that there's a way for us to do anything here. As mentioned in #10686 (comment), targeting the NavigateFunction name itself does work in this report:

"@typescript-eslint/no-floating-promises": ["error", {
    "allowForKnownSafeCalls": [
        { "from": "package", "name": "NavigateFunction", "package": "react-router" }
    ]
}]

Closing out as non-actionable. Thanks all!

@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Feb 3, 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 Feb 11, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. triage Waiting for team members to take a look
Projects
None yet
Development

No branches or pull requests

4 participants