Ok so this one took me forever to simplify to the point we can see what is happening here, although it is still a pretty confusing bug which is already extremely nasty. It only shows itself in very specific situations and causes confusing issues which are often masked as different bugs completely!
This issue is definitely avoidable - but the end result shows that there is a need to handle this situation in a better way.
When using async/await there are times that it ends up returning the Promise
class as the return type. Meaning that in order to use the return value in this case as an object of any form, you need to also refine !val instanceof Promise
.
Many times it doesn't actually show that this is happening. I haven't been able to refine the specific cases I have run into but it ends up happening when this error compounds onto others and Flow decides to only show the errors that this causes rather than the underlying issue itself.
Note: This issue also will ONLY occur when importing. Any repro attempt using the
Try
playground is going to work fine from what I can tell.
Essentially the end result looks like this (note the examples here are all over simplified)
We are programming and all is well, we inspect return types and we like what we are seeing:
Then all of the sudden things get problematic. We start seeing errors about Promises and unions:
This type is incompatible with the expected param type of
union: type application of class `Promise` | type parameter `T` of await
---
Member 1:
type application of class `Promise`
---
Error:
object type
---
This type is incompatible with
object type
---
Property `foo` is incompatible:
Member 2:
type parameter `T` of await
---
Error:
property `foo`
---
Property not found in
Promise
---
Upon inspection, if we are lucky enough that this is the error that actually shows, we can see that the following has happened:
It is likely due to the following logic within the definition of $await
which is also mirrored through within Promise
(although it doesn't appear to occur wh
declare function $await<T>(p: Promise<T> | T): T;
This kind of logic is used throughout and the problem seems to come in when refinement causes it to return the second value within the union rather than the first (and the result is Class<Promise>
)
So the issue seems to occur when a few parameters are met:
this
, another object, etc) with an annotation./* @flow */
import test from './test';
type Blah = {
foo: string,
};
const obj: { val?: Blah } = {};
async function start() {
const val = await test();
if (val != null && typeof val === 'object' && typeof val.foo === 'string') {
obj.val = val;
}
}
then from ./test.js
, return any kind of promise.
/* @flow */
export default function foo(): Promise<{}> {
return Promise.resolve({
foo: 'bar',
});
}
While the reason for this becomes obvious with this simple example, the issue ends up showing itself in extremely nasty and hard to debug situations.
In this case it is because the annotation returning the promise clearly only shows {}
and doesnt perfectly match the given object.
However, this behavior only occurs when all of the given conditions are met and creates inconsistent handling. Using .then()
will never show this bug (that I have experienced) so every time that it has occurred it has ended up been very very confusing.
It took awhile to track down these specifics.
So while it isn't really a bug, I have seen quite a few examples of people reporting issues that are likely related to this specific issue.
It should probably either be made to be much more consistent or to provide an error which is easier to understand what and why the issue is happening.
In my case it was happening when trying to type objects which will be highly variable and trying to maintain coverage as much as possible.
I think this relates to your return being Promise<{}> when it should be Promise<{[key: string]: any}>
Nope, in my original implementation I use a variant of that. Also, using any = lost coverage. Our projects have a fairly strict 100% coverage rule minus a few edge cases.
It simply has to do with union handling and refinement issues. It mostly makes sense what it is doing and why, but it is a problem in terms of user confusion and how it is reported. Promises, being asynchronous, tend to introduce a lot of complexity to such things anyway.
Running into this issue more and more as I build things up. It appears that if you return ANY object through a Promise w/ async/await
it will also return Class<Promise>
, breaking code.
For example, I have a class which directly returns:
send = async (...args: Array<any>): Promise<PubChan$EmitResponseRef> => { ... }
When I try to use it:
async function collectResults() {
const ref = await chan.emit('bar', 'foo').send();
const { results } = ref;
if (Array.isArray(results)) {
for (const result of results) {
console.log('Result: ', result);
}
}
}
It will cause errors.
Which appear to be the same issue, it is returning Class<Promise>
as a possibility.
Whereas if I do not use await, no errors:
It can be fixed in this situation by simply changing $await
to the following:
// we use this signature when typing await expressions
declare function $await<T>(p: Promise<T>): T;
However, I can only imagine this is going to break things in many other situations.
Also notable is the fact it appears to lose it's exactness when using
await
@bradennapier I believe i'm experiencing the same thing, not sure if the examples here help, https://stackoverflow.com/questions/47817511/flow-errors-on-promises
I'm surprised that this issue has so few comments, the patchy support for await/async is my number one frustration with flow.
Cant say I've ran into the issues described here, and I'm using Promises everywhere in my code.
There must be something specific being done to trigger the supposed issue, or there is a misunderstanding either one.
Flow's typedef is correct, see in console:
(async function () { console.log(await 4); })();
// prints 4
My only frustration I run into is when I do something wrong and the error message isn't as clear as to what my error is, or where the error sourced.
IE i once had const foo = await bar; when i needed const [foo, err] = await bar;
then when I used foo.something - it triggered errors in other confusing locations. Flow knew something was wrong, but didn't tell me exactly why.
But ive yet to write valid code working with promises that flow complained about.
I am getting the same behavior, when awaiting a method which returns a promise, the resultant type is ExpectedType | Promise
Moreover applying @bradennapier definition of await does fix the return type to no longer include Promise, but it does as he points out break other aspects of the typing.
For now we are working around this by applying the type explicitly at the callsite, i.e.
let foo = await getFoo()
// foo: ExpectedType | Promise
let foo: ExpectedType = await getFoo()
// foo: ExpectedType
This is still completely broken, even the most basic tryflow example doesn't work as of 0.79.1, including attempts with @rt2zz 's workaround or by wrapping the promise locally.
@rakelley that try flow example is wrong
any async method MUST!!! return a Promise
This is valid:
https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVAXAngBwKZgBiccYAvGAN6phgBGAhgE4BcYAzhkwJYB2A5qgC+6AMZxenMPzwZipClACuvURm4SAFAEo2ABSZwAtt3Z4APPIB8VGmCaylTXmF54kB46bybND9nAwAG542uQ21LQAkP6BIZqUjKxgAOSMAF4pQtoA3HbZeUJ5qOKSGGAYeJyEKqIAguRgDOxYqmDKqupaumCeJmaWJBF2DhhOLgwIDNzlMnIkOoXFpVKV1bUAQo3NraLttV28OvqG-RbWtrQwsvZVSjAYbPLbUzPSsvKLdiOOzrfs9wwSzEElWVTktQAwtsWm0OmoNEcen1vIM4MNaKNxq53L1Tt5fLFgngADS3ABWeDUYTIGNo73mcB0ADoMAALPBHIkhbTM0QMDCiNl+PCU6l5WjZYR5IA
Okay, I didn't expect to have to explicitly type the implicit wrapper, but regardless that just shifts the problem one call level up (let somevar = await testFuncA();
is still going to be Promise | Foo
instead of Foo
).
Looking back at the original complaint, i think this whole thing is without merit.
If your method is marked async, it returns a promise.... no ifs ands or buts.
Try this in the REPL:
(async function() {})()
You will get a Promise.
If you are expecting to write async function getFoo(): Foo {}
, your expectations are wrong.
async function getFoo(): Promise<Foo> {
return new Foo();
}
const foo: Promise<Foo> = getFoo();
const foo: Foo = await getFoo();
That is the only valid code for async functions and it works just fine in flow.
This is where it gets dirty:
let cached: Foo;
function getFoo(): Promise<Foo> | Foo {
if (cached) {
return cached;
} else {
// yay dirty race condition bug here too!
return new Promise((resolve, reject) => {
fooProvider().then((foo) -> {
cached = foo;
resolve(foo);
}).catch(reject);
});
}
}
const maybeFoo: Promise<Foo> | Foo = getFoo();
// We MIGHT of been cached, maybe not? let's guarantee we have a foo:
const foo: Foo = await Promise.resolve(maybeFoo);
You now have a function with mixed return types. You really should avoid this.... just mark the getFoo method async and now your function consistently returns Promise
@rakelley with async keyword, you will never ever have Promise
I take back part of my last statement. I see the OP is mentioning something else, but I can't say I've ran into that scenario.
But as I said in January, Flow's typedef is correct.
I tried the OP's suggested broken code and fixed the return type of the test/foo method and it passes without error:
https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoApgDwA5wE4AuYAJulAIYCuMRUlAdgMYECWc9YUccAFAJQAuMAAU8cALYsAzugA8AbwDaAa3QBPIVIJ4W9AOYBdIeXpqAvgD4w81GDB50BSng6iJ09ADoHUuDABu6Dw2dnZccEIA5ABG5HiRADS2YGZ8ANyoZqioBGrY6GAAQjDkABZgALzWyeGa2rp6SWYZqIzsWmBw0QBWQvJg-uQwAPxCxWUpldbN2eRSakycDMxsHFpxBPzVdm30HYMwU+QI5Cy03PwZdixQYDwHYACEVfTUhwBk72C5+XC3DxVAWBIl1uuhmJEwJ9vnl0H8BkNPOFKkDIlodPpInxtqFQZ4AQiYFcUpkgA
Anyone getting here through google, the solution for me was that i needed to define the type of return in the function i was await-ing:
async someFunction(someVar: string): Promise<Object> {}
Given that the method was to return an Object from the promise. This took away the errors for me.
FYI Still running into this on a completely separate situation. Again its that await
returns a possible promise which is impossible since it will resolve to a value not a promise 100% of the time.
Running into this situation as well using node-pg.
query.js
// @flow strict
import { Pool } from "pg";
import type { ResultSet } from "pg";
const pool = new Pool();
export default (text: string, params: Array<any>): Promise<ResultSet> =>
pool.query(text, params);
test.js
// @flow strict
import query from "./query";
export async function runQuery() {
const test = await query("SELECT * FROM TABLE;", []);
return test;
}
Flow seems to think test
is of type Promise<ResultSet> | ResultSet
, when it should quite obviously be just ResultSet
Yep you need to provide the type in this case (on the result var directly) and it will solve the issue. Not ideal clearly but it has a workaround at least.
Also to those of you mentioning you canβt recreate it in flow try - as I pointed out it ONLY HAPPENS when you import from another file.
In the same file it seems to be fine.
I can confirm, I have similar errors only when multiple files are involved.
I think I have run into this issue.
I have 2 files:
File1.js:
const getThings = async (): Promise<Array<SomeType>> => { ... };
export { getThings };
File2.js:
import { getThings } from 'File1';
const doStuffWithThings = async (): Promise<Array<SomeOtherType>> => {
const things: Array<SomeType> = await getThings();
...
things.forEach(thing => { ... });
...
}
Flow complains that Either cannot call things.forEach because property forEach is missing in Promise
.
Changing the return type of getThings
to Promise<any>
makes the error go away though.
I think it would be really helpful if somebody put up a gist with a complete example that exhibits the unexpected behavior. Unless I missed something, all I've seen on this issue are snippets from actual codebases, or other partial examples.
I've seen what I think is the same issue during my own use of Flow. It's always been a legitimate error, though, which additional type annotations helped to reveal. If I'm correct, this issue is about opaque error messages, and not about Flow erroring when it should not. A concrete example would help determine whether I'm correct.
Thanks @nmote ! I believe you are 100% correct, the issue happens if you have any accesses on the value which are not in the expected return type in the Promise
, and I can see exactly why this happens if you look at the definition of $await
.
I believe the best solution would be for flow
to understand that a call using await
can never result in the Promise<>
part of the union which should then simultaneously fix the opaque error messaging.
In all my situations so far it does end up being an error on my part - although sometimes very hard to find as it can be a random property access at nearly any point after that since flow is generally fairly smart at these things.
Another key point is it only appears to happen if you are importing the async function
from another file.
The typedef signature is correct and does not need to be changed.
The problem is that in same file the await signature of Promise<T> | T
maps the input to Promise<T>
(which gives T
the correct value) and other-file usage maps to both, giving both as a potential response.
@bradennapier I state this in hopes that it makes what the true nature of this bug is, to be more clear, as you tacked on the note about 'another file' as 'another' key note, but that is the only key note that matters here.
I believe the best solution would be for
flow
to understand that a call usingawait
can never result in thePromise<>
part of the union which should then simultaneously fix the opaque error messaging.
This is resolved by ensuring that the input maps toPromise<T>
and not the| T
portion, as it currently does within the same file.
My assumption is that it the passed argument to the parameter is able to 'accurately' identify it matches the Promise
@nmote does that help? Sorry I don't have time to make a gist atm though.
@nmote I think you're right that this about 'opaque error messaging'. In all my attempts to make a small repro case, flow check --show-all-branches
correctly identifies the actual location of the problem (in one of the extra shown branches), but the primary error is always attributed to 'cannot call await because this or that type is incompatible with Promise'.
See https://gist.github.com/akrieger/f1aa74fd5b5819cef96c100fc034d63b for an example. I've commented in index.js the place where the error is introduced and how to resolve it, and described how that changes flow's behavior.
@akrieger we had a similar issue with this "red herring" error message, and flow check --show-all-branches
helped us. Thank you π
Our example minimally was this:
// file1.js
type Response = {
propA?: boolean,
propB?: string,
}
export async function anAsyncFunction(input: any): Promise<Response> {
// stuff
}
// file2.js
import { anAsyncFunction } from './file1.js'
async function doSomeWork() {
...
const { propA = false, propB } = await anAsyncFunction('some input')
...
const templateLiteral = `here's a string with ${propB} interpolated`
...
}
When running flow
, the error we saw was:
Cannot call await with anAsyncFunction(...) bound to p because property propA is missing in Promise [1].
However, after running flow check --show-all-branches
, we then saw this error plus the real error:
Cannot call await with anAsyncFunction(...) bound to p because:
β’ Either cannot coerce propB to string because undefined [1] should not be coerced.
β’ Or property propB is missing in Promise [2].
The fix was to either add a sensible default when destructuring the response, like:
const { propA = false, propB = '' } = await anAsyncFunction('some input')
or use the String()
global to force string in the template literal:
const templateLiteral = `here's a string with ${String(propB)} interpolated`
I think this is a bug bc propA
was in fact defined in the Promise's return value. Why did flow "choose" the wrong error?
@kweiberth propB
is specified as an _optional_ property, so it can be either a string or missing, i.e. undefined
. You shouldn't implicitly coerce undefined
to a string because that strongly suggest that you aren't aware of its type and that you're making a mistake. The error goes away if you cast it into a string with String
because then you're being explicit about your intentions. It also goes away when you default to an empty string, because then it can't be undefined
, therefore it can be interpolated.
Hi @silvenon that makes a lot of sense. I actually mistyped on the last line and I've since updated it. The issue I see here is that the error message that flow originally gave was that propA
type declaration was missing, when it wasn't. The real error it should've been showing was the propB
undefined/coercion error that you described, which was displayed to me only after running flow check --show-all-branches
I can confirm that declaring the expected return type on the variable seems to clear it up.
i.e. do this:
const thing: Thing = await getThing()
not this:
const thing = await getThing()
@cappslock hm, that doesn't seem right, are you sure that you didn't lose the type for getThing
or getThing()
somewhere, so it ended up being any
? If you do const thing = await getThing()
, what is the type of thing
?
This is a silly issue to have :)
@wi-ski this can't be worse than this one https://github.com/facebook/flow/issues/2093
This seems easy to fix, just replace $await
definition with this
declare function $await<T: Promise<any>>(p: Promise<T>): $Call<typeof $await, T>;
declare function $await<T>(p: Promise<T>): T;
declare function $await<T>(p: T): T;
This is also just better definition overall, because it makes Promise<Promise<Promise<Promise<number>>>>
to number
Well, with mine definition, Flow tests on await
just hangs :\
Also it breaks another test, well that's too bad
Thank you @kweiberth! the issue was indeed that a type was being coerced a few lines after... still the error message is super confusing.
I was able to simplify the the reproduction to a single index.js
file, plus a standard config, on the latest version of flow. https://gist.github.com/terite/47e5d725bae07723fb11ea5b4a470a65
I've tried to demonstrate that this error
any
.ignore my last reply, i re-interpreted it the report.
ultimately its because you forced flow to pick promise return BECAUSE the left hand side is wrong
Only way i can see fixing this to identify the users error is to explicitly look for explicit Promise Left Hand Side types of an await.
await
is one common way to trigger this bug, but the fact that imports are a necessary step in reproduction hints that something more interesting is going on.
I've added two.js
to my gist which demonstrates a variation of this bug which is not dependent on async/await/promises. I believe something similar is probably possible using $Call
.
https://gist.github.com/terite/47e5d725bae07723fb11ea5b4a470a65#file-two-js
This appears to have been fixed!
When I run a flow
built from a recent version from master (f1a694ef10a5b27a18cdb0c790ce48e683dcb72b) on @terite's helpful repro, the error messages are in all the right places and none of the wrong ones:
Full error output
Error βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ index.js:11:23
Cannot assign await fs.promises.stat(...) to b because Stats [1] is incompatible
with number [2]. [incompatible-type]
index.js
8β const a: Stats = await fs.promises.stat('/')
9β
10β // good: flow forbids specifying an incorrect type
[2] 11β const b: number = await fs.promises.stat('/')
12β
13β // BUG: missing error here, await will not return a promise
14β const c: Promise<Stats> = await fs.promises.stat('/')
/tmp/flow/flowlib_502b8788d4ba690b_1000/node.js
[1] 1331β stat(path: FSPromisePath): Promise<Stats>,
Error βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ index.js:14:31
Cannot assign await fs.promises.stat(...) to c because Stats [1] is incompatible
with Promise [2]. [incompatible-type]
index.js
11β const b: number = await fs.promises.stat('/')
12β
13β // BUG: missing error here, await will not return a promise
[2] 14β const c: Promise<Stats> = await fs.promises.stat('/')
15β }
16β
17β async function testAwaitNonPromise() {
/tmp/flow/flowlib_502b8788d4ba690b_1000/node.js
[1] 1331β stat(path: FSPromisePath): Promise<Stats>,
Error ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ index.js:23:7
Cannot get b.then because property then is missing in Number [1]. [prop-missing]
[1] 4β const F_OK: number = fs.F_OK
:
20β
21β // good: flow knows awaiting non-promises does not make them promises
22β const b = await F_OK
23β b.then // error is here, in the correct spot
24β
25β // good: flow does not get confused if the awaited value is external, but not imported
26β const c = await Uint8Array.length
Error ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ index.js:27:7
Cannot get c.then because property then is missing in Number [1]. [prop-missing]
index.js
24β
25β // good: flow does not get confused if the awaited value is external, but not imported
26β const c = await Uint8Array.length
27β c.then // error is here, in the correct spot
28β
29β // BUG: flow gets confused if the awaited value is imported
30β const d = await fs.F_OK // error is here, the wrong spot
/tmp/flow/flowlib_502b8788d4ba690b_1000/core.js
[1] 347β length: number;
Error ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ index.js:31:7
Cannot get d.then because property then is missing in Number [1]. [prop-missing]
index.js
28β
29β // BUG: flow gets confused if the awaited value is imported
30β const d = await fs.F_OK // error is here, the wrong spot
31β d.then
32β }
33β
/tmp/flow/flowlib_502b8788d4ba690b_1000/node.js
[1] 1201β declare var F_OK: number;
Error ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ two.js:13:7
Cannot get a.length because property length is missing in Number [1]. [prop-missing]
[1] 10β const a = assertNotNull(5)
11β
12β // good, flow knows number has no length
13β a.length
14β }
15β
16β function testImportedValue() {
Error ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ two.js:19:7
Cannot get a.length because property length is missing in Number [1]. [prop-missing]
two.js
16β function testImportedValue() {
17β // BUG: flow gets confused if the provided value is imported
18β const a = assertNotNull(fs.F_OK) // error is here, the wrong spot
19β a.length
20β }
21β
/tmp/flow/flowlib_502b8788d4ba690b_1000/node.js
[1] 1201β declare var F_OK: number;
Found 7 errors
OTOH the issue reproes if I use v0.141.0, the latest release.
In fact, that commit and v0.141.0 have just a very few differences:
$ git log --oneline --graph --boundary v0.141.0...f1a694ef1 --cherry-mark
= d8b5711f5 (HEAD, tag: v0.141.0, upstream/v0.141) Flow v0.141.0
= 1a6b46555 cache resolved repositioned OpenTs
= 40b9ba2da fix flow error in packages/flow-dev-tools
= ece39a167 add Find_documentation.def_loc_to_comment_loc_map
= b1fdafeb3 don't insert redundant = in JSX autocomplete when attribute has value
= 0c4eae812 [easy] switch Autocomplete_js.Acjsx to record argument
* cc9779cba Back out "[Flow] Refactor Merge_js"
* bd8789826 Back out "[flow] merge imports before check"
* 9ee4de48c Back out "[Flow] skip post_merge_checks in types-first merge"
| = f1a694ef1 (master) Flow v0.141.0
| = 7970cec5b cache resolved repositioned OpenTs
| = 3a33b21c6 add Find_documentation.def_loc_to_comment_loc_map
| = 033a85194 fix flow error in packages/flow-dev-tools
| * 8efa4d5b6 move tests to "classic-only"
| * 28f5aa893 upgrade tests/{react,relay} to types-first
| * 362ee0e06 upgrade tests/o to types-first
| * 5834f34a4 upgrade tests/more_* to types-first
| * 0ff0dbf98 upgrade tests/lint_cli_* to types-first
| * b746999fc upgrade tests/{fixpoint,demo,implicit_instantiation,type_visitor} to types-first
| = 923d58e4a don't insert redundant = in JSX autocomplete when attribute has value
| = 27142d1ce [easy] switch Autocomplete_js.Acjsx to record argument
|/
o 38d2dcd21 [easy] remove debug print statement from ty_members
which, other than the test suite, are entirely that v0.141.0 has reverted some changes that look related to "types-first".
So it appears that those changes (which were reverted just for that release) fix this issue. (Or perhaps more accurately: that types-first fixes this issue, but reverting those changes meant types-first doesn't apply here.) Hopefully that means that a future release will include those changes, and will fix this bug.
Most helpful comment
I am getting the same behavior, when awaiting a method which returns a promise, the resultant type is
ExpectedType | Promise
Moreover applying @bradennapier definition of await does fix the return type to no longer include Promise, but it does as he points out break other aspects of the typing.
For now we are working around this by applying the type explicitly at the callsite, i.e.