TypeScript
TypeScript copied to clipboard
microsoft
Prevent spreading of promises, as it's likely not intended
Suggestion
Spreading a promise in an object is correct Javascript, and Typescript allows it. But it doesn't do anything:
{ ...Promise.resolve({ key: 42 }) } // returns {}
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.
const myAsyncFunc = () => Promise.resolve({ key: 42 });
const result = {
foo: 'bar',
...myAsyncFunc(),
};
While this is "correct", it's almost certainly not what the programmer intended.
🔍 Search Terms
Spreading promise.
✅ Viability Checklist
My suggestion meets these guidelines:
- [ ] This wouldn't be a breaking change in existing TypeScript/JavaScript code
- [x] This wouldn't change the runtime behavior of existing JavaScript code
- [x] This could be implemented without emitting different JS based on the types of the expressions
- [x] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
- [x] This feature would agree with the rest of TypeScript's Design Goals.
(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.
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.
Ah, wait, promises don't have any own properties whatsoever—everything's on Promise.prototype. So instead, my comment should be a feature request for TS to understand the prototype chain 😄
TS has a long way to go
I think the specific case of disallowing spreading a Promise is very unobjectionable -- it's extremely obviously a mistake, and a refactoring hazard when asyncifying a sync function.
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 opts gets subtype-reduced to { } and thus has zero enumerable properties.
Initially, I thought this was something that would have to be integrated with Typescript, but typescript-eslint can indeed prevent this error. I made a change in this PR. So as soon as that gets published, there should be a good way to work around the issue described here.
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 tsc when it should not. When commenting or awaiting ...randomPromise(),, it correctly displays the error:
src/index.ts:7:9 - error TS2322: Type 'number' is not assignable to type 'string'.
7 a: 1,
~
Found 1 error in src/index.ts:7
