-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Prevent spreading of promises, as it's likely not intended #49161
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
Comments
+1 |
This can be done in a much more generic way if we can tell TS that certain keys are non-enumerable. Spreading an object with all keys non-enumerable is likely a mistake. But... sigh there are a lot of JavaScript nuances that TS can't capture. |
Issue for what @Josh-Cena mentioned: #9726 |
Ah, wait, promises don't have any own properties whatsoever—everything's on TS has a long way to go |
I think the specific case of disallowing spreading a A rule of "disallow spreading if there are zero enumerable properties" is overbroad, because people will sometimes write something like const opts = useOptions ? options : { };
const x = { ...opts, foo: 3 } wherein |
I guess there should be an eslint rule to forbid spreading promises |
Initially, I thought this was something that would have to be integrated with Typescript, but |
I just experienced a weird situation where accidentally spreading a promise would disable the type checking for the return type of an async function. I had to dig into it to find this issue and figure out that there was an Eslint rule for this that wasn't enabled because of a misconfiguration on my end. Here is the code snippet: async function randomPromise(): Promise<Record<string, string>> {
return { b: "2" };
}
export async function test(): Promise<Record<string, string>> {
return {
a: 1,
...randomPromise(),
};
} The above snippet passes
|
Suggestion
Spreading a promise in an object is correct Javascript, and Typescript allows it. But it doesn't do anything:
Any time you're trying to spread a promise, it's much more likely that you have an async function that you forgot to await. The following code compiles.
While this is "correct", it's almost certainly not what the programmer intended.
🔍 Search Terms
Spreading promise.
✅ Viability Checklist
My suggestion meets these guidelines:
(I'm not sure about the first point. This would be a breaking change, but it should only affect code that likely contains a bug. The feature request does support the goal of "Statically identify constructs that are likely to be errors.")
⭐ Suggestion
Prevent code that spreads promises from compiling.
📃 Motivating Example
As it's almost certainly unintended, Typescript should prevent promises from being spread in objects.
💻 Use Cases
This would prevent a bug that I personally faced recently. I can't think of any scenario where someone would intentionally want to spread a promise.
An IDE or a plugin could detect that you're spreading a function marked as
async
, but I'm not sure whether they would be able to detect that a function returns a promise, or that a function returns the result of another function call that returns a promise.The text was updated successfully, but these errors were encountered: