-
Notifications
You must be signed in to change notification settings - Fork 107
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
Use of await
and yield
#66
Conversation
This is really nice. Does this mean it's possible grammar-wise? What would happen in this case? Syntax error? var result = 10 |> g |> await; |
@gilbert Possible? Requires review, but I'm pretty sure it is. Your example? Yes, an error because a function expression would be expected, given the last Your thoughts? |
I mentioned in #53 (comment), but I think this could be confusing. userId |> await fetchUserById |> getAgeFromUser
Something like userId |> fetchUserById |> getAgeFromUser(await ?) makes more sense to me because it would expand to userId |> fetchUserById |> promise => getAgeFromUser(await promise) which is the same as getAgeFromUser(await fetchUserById(userId)) |
It's not a normal type of closure ( Furthermore, it reads fairly naturally to me, in order: userId |> await fetchUserById |> getAgeFromUser "Pass I'm not sure how your alternative should be read and understood? |
Some more examples to consider: var A = fetch(url) |> handleFetchPromise;
var B = await fetch(url) |> handleFetchPromise;
var C = ( await fetch(url) ) |> handleFetchPromise;
var D = await (fetch(url) |> handleFetchPromise); Which are equivalent, which are not? |
Personally, I would have B equivalent of D (that is, Your and anyone else's thoughts? |
Given |
I think this change would make the pipe operator too complicated. I would rather see the pipe operator stay simple and easy for people to build a mental model of, and see the async case handled by some new |
Rejecting the ability to await async function results in a pipeline would make it less capable than ordinary |
There's no need for a "waterfall" when arrays have a reduce method, and async functions can use loops; I'm not sure why that would be helpful. |
@TheNavigateur it would make the pipe operator more complex, which I am not ok with. I'd rather people had to do |
@TehShrike Your primary concern is not affected by this change, and would exist anyway: But the larger question is: you want to force people to have to do |
If your understanding of The pipe operator, in JS or any other language, is great for composing a bunch of transformation functions. Adding generator/promise awareness on top of that is not necessary. |
Let's examine your claim that this is a "leaky" abstraction: I would claim it is a rock-solid abstraction. However, perhaps you can give an example of where this isn't the case. |
Maybe this is a job for |
I know most of the discussion here is around Nothing screams "don't do this!" about including |
@littledan Usage example? @mAAdhaTTah I'm simply proposing making it as capable as the existing |
Throwing my 2¢ in: var A = fetch(url) |> handleFetchPromise;
var B = await fetch(url) |> handleFetchPromise;
var C = ( await fetch(url) ) |> handleFetchPromise;
var D = await (fetch(url) |> handleFetchPromise); This is how it parses today in Babel (and it all makes sense to me).
I think the more interesting cases are awaiting after a var E = fetch(url) |> await handleFetchPromise;
var F = fetch(url) |> await handleFetchPromise |> next;
var G = fetch(url) |> handleFetchPromise |> await next;
var H = await (fetch(url) |> handleFetchPromise); Currently, babel awaits for the function reference that will be invoked (not the actual invocation of it). This is not what I'd want when writing it, and I agree with the PR's proposed semantics. Specifically, it'd be a syntax for
We can wait on the expression var I = fetch(url) |> (await handleFetchPromise); This breaks the |
@jridgewell That's a coherent proposal, and pretty intuitive. I suppose we could specify that by putting a nolookahead for await and yield after var E = fetch(url).then(handleFetchPromise); Maybe it's not so easy to understand |
@jridgewell Thanks for your input. If I understand correctly, one thing that strikes me as odd is the relationship between B and E: var B = await fetch(url) |> handleFetchPromise;
var E = fetch(url) |> await handleFetchPromise;
// Are B and E equivalent?
// What about J?
var J = await fetch(url) |> await handleFetchPromise; I think it would be ideal for an var B = await fetch(url) |> handleFetchPromise;
//=> handleFetchPromise(await fetch(url))
var E = fetch(url) |> await handleFetchPromise;
// => await handleFetchPromise( fetch(url) )
var F = fetch(url) |> await handleFetchPromise |> next;
//=> next(await handleFetchPromise( fetch(url) ))
var G = fetch(url) |> handleFetchPromise |> await next;
//=> await next( handleFetchPromise(fetch(url)) )
var H = await (fetch(url) |> handleFetchPromise);
//=> await handleFetchPromise(fetchUrl(url))
var J = await fetch(url) |> await handleFetchPromise;
//=> await handleFetchPromise(await fetch(url)) And your rule about "breaking the |
@gilbert Yes, B and E should be equivalent, in my opinion. J should be different (awaits the fetch(url) instead of synchronously passing the promise). As for your examples from E to J, that's what the PR recommends. Your |
@TheNavigateur B is consistent with how it parses in Babel today, as @jridgewell explained. It's also consistent with the F example I wrote, which behaves the same way. |
@gilbert B is not exactly consistent with F: F doesn't glue Whereas, B glues Basically, if we put |
You might be misreading the code I wrote, so let me compare @jridgewell's suggestion to my suggestion side by side: // @jridgewell's
var F = fetch(url) |> await handleFetchPromise |> next;
//=> next(handleFetchPromise(await fetch(url)))
// @gilbert's
var F = fetch(url) |> await handleFetchPromise |> next;
//=> next(await handleFetchPromise( fetch(url) )) (our Bs are the same) The F example I wrote does make |
I think in general, yes, anything can be written as
That makes more sense than my proposal, I like it. In this case, |
In
So I think simply giving await x |> f is the equivalent of await (x |> f) (I am) Otherwise, you would need to give a higher precedence to I think that simply giving |
When replacing promise chains without introducing unnecessary bindings? async function rejectIssues(user) {
return getIssues()
.then(filterByCreator(user))
.then(assignLabelsToIssues(["wontfix"]))
.then(closeIssues)
} Current translation: async function rejectIssues(user) {
return closeIssues(
await assignLabelsToIssues(["wontfix"])(
await filterByCreator(user)(
await getIssues()
)
)
);
} With pipeline operator: async function rejectIssues(user) {
return getIssues()
|> filterByCreator(user) |> await
|> assignLabelsToIssues(["wontfix"]) |> await
|> closeIssues;
} |
Or, alternatively, with the other pipeline operator PR, as well as the partial application proposal rather than the "elixir option", async function rejectIssues(user) {
return getIssues()
|> await filterByCreator(?, user)
|> await assignLabelsToIssues(?, ["wontfix"])
|> closeIssues;
} |
@littledan Advantage of |
I'm fine with await readFile("foo.txt")
|> stripBom
|> await writeFile("foo.txt") If const file = await readFile("foo.txt")
|> stripBom;
await writeFile("foo.txt") // WRONG!
// actually you want:
// await writeFile("foo.txt")(file) Worse, it causes complications when moving some code into a pipeline. /** @returns {Function} */
async function getResolver() {
const { type } = await readConfig();
return resolvers[type];
}
// original
(await getResolver())(query);
// attempt 1
query |> await getResolver(); // wrong, this is `await getResolver()(query)`
// attempt 2
query |> await getResolver; // wrong, this is `await getResolver(query)`
// attempt 3
query |> (await getResolver()); // correct |
I'm not comfortable with
That made me think of maybe another alternative:
(Replace the |
@azz I'm not sure what you mean in your example. Are you imagining that pipelining puts the left hand of the operator into the first argument position of the right hand, or are you imagining that something like |
I was assuming the functions were curried. |
@Volune For me, since |
Yes, it's true that the I'm a bit uncomfortable with |
@littledan Do you envisage any disadvantages with |
@TheNavigateur I think it's a little odd that you can't use it in a trailing way. Trailing |
I would rather see the pipe operator not support |
Should we ban the cases where |
I still think it's easiest not to support let res = (await readRequest())
|> await parseContent
|> await retrieveFromCache
|> await capitalizeOnServer
|> JSON.stringify Can be (assuming function lift(fn) {
return async function lifted(...args) {
let unlifted = await Promise.all(...args);
return await fn(...args); // assume no `this`, can change to `.call` according to proposal
}
}
// assuming `lift` was called on all the above functions:
let res = await readRequest()
|> parseContent
|> retrieveFromCache
|> capitalizeOnServer
|> stringify Which means you never have to await in a promise pipeline. Now would be a good time to point out that I see F# had a similar proposal - let's try summoning @WallaceKelly for their opinion about it. |
@littledan const
user =
lastComment
|> userIdFromComment
|await> fetchUserById |
An alternative, is to allow Otherwise I think an operator like e.g. |
OK, I will prepare a PR which simply bans |
@littledan Did I satisfactorily address or miss your point about the trailing await? |
I'm not sure communication is enough. I will raise the issue to TC39 and present a range of options. |
I think it's important that we keep the possibility of extension in the future. As for yield, I can see how it would be useful in a pipeline but not in real world scenarios and I think we should consider banning it in the middle of a pipeline (for starters) too. |
Given problems with all possibilities considered for await integration with pipeline, ban |> await within an async function as the initial option. When we have considered things further, we could add await integration as a follow-on feature. Relates to tc39#66
Given problems with all possibilities considered for await integration with pipeline, ban |> await within an async function as the initial option. When we have considered things further, we could add await integration as a follow-on feature. Relates to tc39#66
Use of
await
andyield
, that seems the most natural to me, with explanation.