meta(eslint-config): Don't allow voiding Promises#9814
Conversation
|
@lforst Can you share an example? |
size-limit report 📦
|
| }); | ||
| // This should never reject | ||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
| this._worker.postMessage('clear'); |
Lms24
left a comment
There was a problem hiding this comment.
I think this is a good idea.
Not sure msyelf but wdyt about allowing voiding in tests?
@Lms24 I had the same thought but for some reason I couldn't get the eslint filtering rules to properly apply so I just said "screw it im ignoring all of these"... Let me try again. |
|
Ok I don't think this is is easily doable because eslintrcs look from the root and in "test packages" like e2e tests there are no test folders that we can match. Test folders and test files are already excluded from this rule. |
Lms24
left a comment
There was a problem hiding this comment.
Ok I don't think this is is easily doable because eslintrcs look from the root and in "test packages" like e2e tests there are no test folders that we can match. Test folders and test files are already excluded from this rule.
makes sense, thx for trying!
|
|
||
| void run(); | ||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
| run(); |
There was a problem hiding this comment.
Wdyt?
| run(); | |
| (async () => { await run() })(); |
There was a problem hiding this comment.
This will still create a floating promise
We should be a bit more strict about floating promises in the SDK. Right now we're just slapping
voidon all of them which is a bit likets-ignore. Ideally, we are a bit more explicit about handling rejected promises or ignoring the lint rule with a description.This pr turns off the "void" option of the lint rule so that we become more explicit.