Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Rule Suggestion: Disallow assignment of () => Promise<...> to () => void parameter or property #4463

Closed
asgerhallas opened this issue Jan 15, 2019 · 3 comments

Comments

@asgerhallas
Copy link

asgerhallas commented Jan 15, 2019

Rule Suggestion

Is your rule for a general problem or is it specific to your development style?

General, I guess :)

What does your suggested rule do?

TypeScript allows me to assign a function that returns Promise to variables or parameters that are type (...) => void. Like:

async function a() { return 0; }
type b = () => void;
const c: b = a;

and:

function d(f: b) { }
d(a);

(see spec 3.11.4)

I suggest a rule that disallows such assignments specifically for functions that return promises, so the caller of the given function does not unintentionally ignore the returned promise.

List several examples where your rule could be used

Given a React component that has props like:

MyList extends Component<{ addItem: (item) => void }> { ... }

MyApp = () => <MyList addItem={saveItem}>
saveItem = async (item) => { 
   // await save to database 
}

If the database call fails this will result in an unhandled promise rejection. To be safe either MyLists props has to be changed, so the given function must return a promise, or saveItem should be changed to handle the error locally.

I must admit I have no more examples right now, but for large React apps these errors can be quite hard to find and thus it might be worth a rule.

@ajafff
Copy link
Contributor

ajafff commented Jan 15, 2019

A common example of people running into this issue is passing an async function as callback to Array.prototype.forEach. Since forEach doesn't care about the return value at all, there's nothing awaiting the returned promise.

If you don't want to wait for TSLint to add such a rule, you can use the one I wrote for my own linter runtime Fimbullinter: async-function-assignability

@asgerhallas
Copy link
Author

@ajafff Thanks, this looks great, I'll look into it :)

@JoshuaKGoldberg
Copy link
Contributor

Very much agreed with this use case! Duplicate of #3983.

The discussion there is a bit muddled but the tl;dr is that yes, this rule would be great.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants