Include async functions in origins promise detection #67
Conversation
oshoemaker
force-pushed
the
os/bug/fix-async-detection
branch
from
faf4415
to
35e1897
Compare
|
@oshoemaker can you add a test for this bug fix? |
@fengmk2, I am not really sure how this can be tested. In our case we are using serverless web pack with target ES2018 and ESNext lib. The issue is that the test would succeed/fail based on node version/transpiler settings. So I can incorporate a test but would need to change the test environment in order to actually test this. Suggestions? |
oshoemaker
force-pushed
the
os/bug/fix-async-detection
branch
3 times, most recently
from
340a8eb
to
56ab6cf
Compare
oshoemaker
force-pushed
the
os/bug/fix-async-detection
branch
from
56ab6cf
to
f824927
Compare
|
I have added a "test" to force it to use the code and resulting output. I am not really sure if there is any other option with the current test configuration. Happy to hear suggestions though. |
|
@fengmk2, any update on this? |
| @@ -56,7 +56,8 @@ module.exports = function(options) { | |||
| let origin; | |||
| if (typeof options.origin === 'function') { | |||
| origin = options.origin(ctx); | |||
| if (origin instanceof Promise) origin = await origin; | |||
| const isAsync = origin.constructor.name === 'AsyncFunction'; | |||
There was a problem hiding this comment.
I'm not sure why we need this detect? When we call an async function, it will return a promise instance.
There was a problem hiding this comment.
@dead-horse , not always, the async function return a promise instance.
const p = new Promise(()=>{});
p instanceof Promise // true
const a = async function () {}
a instanceof Promise // false
a.constructor.name === "AsyncFunction" // true chrome console
There was a problem hiding this comment.
const a = async function () {}
const result = a()
result instanceof Promise // true
Current promise detection does not properly detect async functions in ES2018.