Closed karol-majewski closed 6 years ago
Should we require that the function return a Promise? Greenlet wraps invocation in a promise chain, so synchronous returns are actually supported as well.
@developit Good point. Unfortunately, making it work with synchronous functions is not a trivial task as of TypeScript 2.6.2 with its limited means for extracting function argument & return types.
By requiring an async function we can simply pass its signature down. For regular ones, we'd have to preserve the arguments' types, but promisify the return type.
This would require typing each overload separately, and even that solution has disadvantages: the argument's names are lost, and it's tricky to get optional parameters to work — overloads are chosen based on the arity of your function, and that is constant in our case. You'd have to provide generic types yourself, and while it may be accepted by TypeScript users, it's not possible for JavaScript developers trying to get IntelliSense. See the example here%3A%20(arg1%3A%20A1)%20%3D%3E%20Promise%3CR%3E%3B%0D%0Adeclare%20function%20coworker%3CR%2C%20A1%2C%20A2%3E(fn%3A%20(arg1%3A%20A1%2C%20arg2%3A%20A2)%20%3D%3E%20R%20%7C%20Promise%3CR%3E)%3A%20(arg1%3A%20A1%2C%20arg2%3A%20A2)%20%3D%3E%20Promise%3CR%3E%3B%0D%0Adeclare%20function%20coworker%3CR%2C%20A1%2C%20A2%2C%20A3%3E(fn%3A%20(arg1%3A%20A1%2C%20arg2%3A%20A2%2C%20arg3%3A%20A3)%20%3D%3E%20R%20%7C%20Promise%3CR%3E)%3A%20(arg1%3A%20A1%2C%20arg2%3A%20A2%2C%20arg3%3A%20A3)%20%3D%3E%20Promise%3CR%3E%3B%0D%0A%0D%0Acoworker(add)(1%2C%202)%3B%20%2F%2F%20%3D%3E%20Works%2C%20but%20the%20arguments'%20names%20are%20%22arg1%22%20and%20%22arg2%22%20instead%20of%20original%20ones.%0D%0A%0D%0Afunction%20pff(word1%3A%20string%2C%20word2%3F%3A%20string)%20%7B%0D%0A%20%20%20%20return%20(word2%20%3D%3D%3D%20undefined)%0D%0A%20%20%20%20%20%20%20%20%3F%20word1%0D%0A%20%20%20%20%20%20%20%20%3A%20word1%20%2B%20%22%22%20%2B%20word2%3B%0D%0A%7D%0D%0A%0D%0Acoworker(pff)('Extra'%2C%20'cheese')%3B%20%2F%2F%20Error%0D%0Acoworker%3Cstring%2C%20string%2C%20string%20%7C%20undefined%3E(pff)('such'%2C%20'verbose')%3B%20%2F%2F%20OK).
This might change in TypeScript 2.8, but that leaves us with the question: what should we do now?
Wow, that's interesting haha. I guess for now let's stick to saying it's only for async functions - that's the far more compelling case, since the outward signature is unchanged when wrapping in greenlet()
.
When will this be available on npm? I think the latest npm version doesn't have the ts definition afaik.
should be now.
By the way, TypeScript 2.8.3
is here, and it's easier to infer the return type of a function. We could lift the requirement of the passed function to be async
.
The bad news is that the arguments' names will have to be altered, so I'm not sure if it's worth it.
@karol-majewski guessing that's not backwards-compatible though? Argument names can be anything, they get compressed via Uglify anyway.
@developit It would add support for synchronous functions next to the existing asynchronous ones on the type level. This would be an extension rather than modification, thus being backward-compatible.
And I was referring to the arguments' names inferred by IntelliSense on the call site for the supplied function. ;-)
This is how it would look like.
@developit What do you think of that? ☝️
Personally I like the idea of requiring the function be async simply in order to ensure its signature doesn't change as a result of being wrapped in greenlet.
This adds IntelliSense and prevents the library consumers from passing synchronous functions.