[no-misused-promises] non-thenable thenable union react component attribute does not accept thenable function

[ThiloAschebrock]

• I have tried restarting my IDE and the issue persists.
• I have updated to the latest version of the packages.
• I have read the FAQ and my problem is not listed.

Repro

{
  "rules": {
    "@typescript-eslint/no-misused-promises": ["error"],
  }
}

interface Props {
  onEvent: (() => void) | (() => Promise<void>);
}

const Component: React.FC<Props> = () => null;

const App: React.FC = () => {
  const handleEvent = async () => {};
  return <Component onEvent={handleEvent} />;
};

export default App;

{
  "compilerOptions": {
    "target": "ESNext",
    "useDefineForClassFields": true,
    "lib": ["DOM", "DOM.Iterable", "ESNext"],
    "allowJs": false,
    "skipLibCheck": false,
    "esModuleInterop": false,
    "allowSyntheticDefaultImports": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "module": "ESNext",
    "moduleResolution": "Node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "noEmit": true,
    "jsx": "react-jsx"
  },
  "include": ["src"],
  "references": [{ "path": "./tsconfig.node.json" }]
}

Expected Result

No linting error since onEvent accepts both a thenable and a non-thenable function.

Actual Result

9:29  error  Promise-returning function provided to attribute where a void return was expected  @typescript-eslint/no-misused-promises`

Additional Information

This bug was introduced in 5.14.0. 5.13.0 ist working fine.

Note that you can circumvent the bug by using

interface Props {
  onEvent: () => void | Promise<void>;
}

instead.

Versions

package	version
@typescript-eslint/eslint-plugin	5.14.0
@typescript-eslint/parser	5.14.0
TypeScript	4.6.2
ESLint	8.10.0
node	14.18.3

[bradzacher]

Wasn't this fixed in #4620, @JoshuaKGoldberg?

[ThiloAschebrock]

To me it rather looks like #4620 introduced this bug.

[JoshuaKGoldberg]

Indeed - this is another edge case I hadn't thought of. 😞

[kachkaev]

What would be the right way forward? Just wonder if I should refactor my React callbacks or temporarily disable the rule.

Downgrading @typescript-eslint/eslint-plugin below 5.14.0 is not ideal because of a warning for TypeScript 4.6.

[bradzacher]

With any issue in opened in this project - it either has a visible progress in the form of an attached PR, or it has no progress.

We are a community run project. The volunteer maintainers spend most of their time triaging issues and reviewing PRs. This means that most issues will not progress unless a member of the community steps up and champions it.

If this issue is important to you - consider being that champion.

If not - please just subscribe to the issue and wait patiently.
Commenting asking for status updates does not bump issue priority in any way and just serves to spam everyone subscribed to the issue.

[kachkaev]

Thanks for your comment @bradzacher and sorry for spamming – I did not mean to. The reason why I asked for some clarification was because I was not sure what the resolution was. Should #4620 be reverted? Or should there be some new if statement in the rule that would distinguish react callbacks from other callbacks? If the latter, what would the check be based on? That’s unclear to me on the product level at this point and it’d be great if someone could help.

Because #4620 is a breaking change in a minor version, my intuition would be to revert it in the next patch and think about a smart solution to the problem later. Once the way forward is clear, I’m happy to try submitting PR!

[bradzacher]

Should #4620 be reverted?

No - we aren't in the business of reverting commits unless they create catastrophic breakages. Given this issue is almost a month old with less than 10 users interactions - it's definitely not catastrophic.

This issue is tagged with "accepting PRs" - the way forward is to fix the bug.

Because #4620 is a breaking change

Lint rule enhancements can cause additional lint errors to be revealed in your codebase. This is not a breaking change as we define it.

---

Reviewing the issue again - the lint rule is actually working just fine here.

You're passing a () => Promise<T> into a position expecting a () => void.
This is dangerous because it means you have created a floating promise - the caller will not wait for your async workload to complete before continuing. This could introduce race conditions and other bugs into your code, and also mean that errors are not correctly handled.

function Component({onClick}: {onClick: () => void}) {
  return <div onClick={() => {
    onClick();
    doSomethingImmediately();
  }} />;
}

<Component onClick={async () => { throw new Error() }} />;
// the error gets lost to the void.

#4623 added a way for you to opt-out of this behaviour granularly using rule config, in case you want to allow the unsafe behaviour:
https://typescript-eslint.io/rules/no-misused-promises#checksvoidreturn

{ checksVoidReturns: { attributes: false } }

[esetnik]

You're passing a () => Promise into a position expecting a () => void.

@bradzacher From my reading of this issue I don't think your explanation is quite correct. In this issue you're passing a () => Promise<T> into a position expecting a (() => void) | (() => Promise<void>). This means that it's the calling function's responsibility to check whether a function returning promise was passed and to handle this case appropriately. But the caller of the function should be free to pass an async function without linter error as the component has indicated it is capable of receiving such a function.

[bradzacher]

@esetnik - I think you've misunderstood the OP.

Passing () => Promise<T> to () => void is a lint error, but passing it to (() => void) | (() => Promise<T>) is not..
Which is expected, as the latter signature correctly identifies that the receiver expects a promise.

[esetnik]

@bradzacher please review again the OP example which was working in v5.13.0 and broke in v5.14.0+. What's really weird is that I pasted this same example into the playground page you linked and it is working without error. But when I test this same example in my code using v5.17.0 I get the same linter error as OP. The only difference is that i'm using typescript v4.6.3 and the playground uses typescript v4.6.2.

interface Props {
  onEvent: (() => void) | (() => Promise<void>);
}

const Component: React.FC<Props> = () => null;

const App: React.FC = () => {
  const handleEvent = async () => {};
  return <Component onEvent={handleEvent} />;
};

export default App;

Note that onEvent is declared as (() => void) | (() => Promise<void>) and yet it generates the following error when an async function is passed:

9:29  error  Promise-returning function provided to attribute where a void return was expected  @typescript-eslint/no-misused-promises`

[esetnik]

@bradzacher just wanted to let you know I tested in v5.19.0 and it's still generating the spurious error. I believe this issue needs to be reopened.

[ThiloAschebrock]

I agree, this issue is still persisting.

[JoshuaKGoldberg]

Fix in review here: #4841

Since it's a bit buried in the thread, bumping that you can disable the new check added in v4.6 with the settings described at https://typescript-eslint.io/rules/no-misused-promises/#checksvoidreturn:

"rules": {
  "@typescript-eslint/no-misused-promises": ["error", {
    "checksVoidReturn": {
      "arguments": false
    }
  }]
}

[esetnik]

@esetnik - I think you've misunderstood the OP.

Passing () => Promise<T> to () => void is a lint error, but passing it to (() => void) | (() => Promise<T>) is not.. Which is expected, as the latter signature correctly identifies that the receiver expects a promise.

@JoshuaKGoldberg I see your fix and that makes a lot of sense. Just curious, do you have any idea why I'm not able to demonstrate this error in the playground?

[esetnik]

Fix in review here: #4841

Since it's a bit buried in the thread, bumping that you can disable the new check added in v4.6 with the settings described at https://typescript-eslint.io/rules/no-misused-promises/#checksvoidreturn:

"rules": {
  "@typescript-eslint/no-misused-promises": ["error", {
    "checksVoidReturn": {
      "arguments": false
    }
  }]
}

I had to use:

"rules": {
  "@typescript-eslint/no-misused-promises": ["error", {
    "checksVoidReturn": {
      "arguments": false,
      "attributes": false
    }
  }]
}

to silence all the situations affected by #4650