Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upawait in parameters #917
Comments
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ljharb
May 14, 2017
Member
While I like this idea, see https://github.com/tc39/ecma262#contributing-new-proposals - I think es-discuss might be a better place to suggest this.
|
While I like this idea, see https://github.com/tc39/ecma262#contributing-new-proposals - I think es-discuss might be a better place to suggest this. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Jamesernator
May 14, 2017
The funny thing is in V8 if you add the flag --harmony_do_expressions await already works in that position if wrapped in a do expression:
// Do expressions just for the sake of demonstrating that the code
// needed for `await` in default parameter is already implemented
// in V8 at least
async function foo(bar=do { await 3 }) {
return bar
}
foo() // Promise { 3 }So yield/await should probably work in the default argument position, the fact it doesn't seems like a spec bug/oversight to me, but that's up for implementors to decide if it needs a proposal or not.
The additional syntax for async function foo(await bar) as much as I've always wanted that exact syntax, is probably better suited to a whole new proposal.
EDIT: Just noticed it's explicitly excluded so it's not a spec bug. Is the current restriction better for optimizations or something?
Jamesernator
commented
May 14, 2017
•
|
The funny thing is in V8 if you add the flag --harmony_do_expressions // Do expressions just for the sake of demonstrating that the code
// needed for `await` in default parameter is already implemented
// in V8 at least
async function foo(bar=do { await 3 }) {
return bar
}
foo() // Promise { 3 }So The additional syntax for EDIT: Just noticed it's explicitly excluded so it's not a spec bug. Is the current restriction better for optimizations or something? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
caitp
May 15, 2017
Contributor
Just noticed it's explicitly excluded so it's not a spec bug. Is the current restriction better for optimizations or something?
We need parameter bindings to be heap-allocated rather than on-stack, so that they can be accessed when the generator is resumed.
Imagine:
async function f(id, account = await accounts.lookup(id), transaction = await
account.mostRecentTransaction()) {
}
f().then(...);
// WALKTHROUGH:
// MOVQ $SP[-2], $HeapBinding($2) << undefined
// JNE account, UndefinedConstant(), &next_binding << Does not jump
// <call accounts.lookup(id)>
// SuspendGenerator($generator);
//
// <stuff happens, and then generator is resumed...>
// MOVQ $GeneratorInput, $HeapBinding($2) << result of accounts.lookup(id)
// MOVQ $SP[-1], $HeapBindings($3) << What is in the stack slot once the generator is resumed??So to work around that, you'd have to copy all of the stack-allocated parameters to the heap before initializing parameters, which would (presumably) be slower. ...Of course, the allocation of the arguments object sort of does this anyways (if not elided).
EDIT: not long after I left that comment (or maybe even slightly before it), those concerns were no longer valid in v8.
We need parameter bindings to be heap-allocated rather than on-stack, so that they can be accessed when the generator is resumed. Imagine: async function f(id, account = await accounts.lookup(id), transaction = await
account.mostRecentTransaction()) {
}
f().then(...);
// WALKTHROUGH:
// MOVQ $SP[-2], $HeapBinding($2) << undefined
// JNE account, UndefinedConstant(), &next_binding << Does not jump
// <call accounts.lookup(id)>
// SuspendGenerator($generator);
//
// <stuff happens, and then generator is resumed...>
// MOVQ $GeneratorInput, $HeapBinding($2) << result of accounts.lookup(id)
// MOVQ $SP[-1], $HeapBindings($3) << What is in the stack slot once the generator is resumed??So to work around that, you'd have to copy all of the stack-allocated parameters to the heap before initializing parameters, which would (presumably) be slower. ...Of course, the allocation of the arguments object sort of does this anyways (if not elided). EDIT: not long after I left that comment (or maybe even slightly before it), those concerns were no longer valid in v8. |
ljharb
closed this
May 15, 2017
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bterlson
May 15, 2017
Member
FWIW, I think this is probably a bug and I'm happy to present on this at the next meeting (although likely actually the July meeting since May is already looking full?)
My recollection is that the current semantics were due to the original spec behaving similar to generators where errors thrown from initializers rather than rejecting a promise. When this was changed, we could have then allowed await in params but didn't. Unfortunately the spec change is not entirely straightforward.
|
FWIW, I think this is probably a bug and I'm happy to present on this at the next meeting (although likely actually the July meeting since May is already looking full?) My recollection is that the current semantics were due to the original spec behaving similar to generators where errors thrown from initializers rather than rejecting a promise. When this was changed, we could have then allowed await in params but didn't. Unfortunately the spec change is not entirely straightforward. |
bterlson
reopened this
May 15, 2017
bterlson
added
needs consensus
spec bug
labels
May 15, 2017
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
anba
Aug 28, 2017
Contributor
I wonder if it'll bother people that a possible await in parameters will be only possible in async function expressions/declarations/methods, but not in async arrow functions?
|
I wonder if it'll bother people that a possible |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
mgol
Aug 28, 2017
@anba This sounds like a big issue to me, one that's going to bite in lots of projects with style guides preferring arrow functions...
mgol
commented
Aug 28, 2017
|
@anba This sounds like a big issue to me, one that's going to bite in lots of projects with style guides preferring arrow functions... |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
caitp
Sep 25, 2017
Contributor
I'm not super excited about the "await just default value" idea.
This seems like a potential footgun, and it's hard to spot by reading code.
async function foo(x = await ResolveLater(3)) { return x + 1; }
// From a quick glance, `x` looks like it should never be a thenable, because of
// the `await` initializer.
foo(); // 4
foo(3); // 4
foo(Promise.resolve(3)); // "[object Promise]1" << okay...I'm happier with the idea of annotating parameters with await, to indicate that the await happens whether it's a default parameter or not.
async function foo(await p = Promise.resolve(3)) { return p + 1; }
foo(); // 4
foo(3); // 4
foo(Promise.resolve(3)); // 4The problem with that is it sort of overloads the meaning of "await", since it behaves differently in a parameter context.
FWIW, I would prefer just requiring the explicit await in the function body, for readability/clarity (with a very minor cost in conciseness)
|
I'm not super excited about the "await just default value" idea. This seems like a potential footgun, and it's hard to spot by reading code. async function foo(x = await ResolveLater(3)) { return x + 1; }
// From a quick glance, `x` looks like it should never be a thenable, because of
// the `await` initializer.
foo(); // 4
foo(3); // 4
foo(Promise.resolve(3)); // "[object Promise]1" << okay...I'm happier with the idea of annotating parameters with async function foo(await p = Promise.resolve(3)) { return p + 1; }
foo(); // 4
foo(3); // 4
foo(Promise.resolve(3)); // 4The problem with that is it sort of overloads the meaning of "await", since it behaves differently in a parameter context. FWIW, I would prefer just requiring the explicit await in the function body, for readability/clarity (with a very minor cost in conciseness) |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
pemrouz
Oct 12, 2017
I wonder if it'll bother people that a possible await in parameters will be only possible in async function expressions/declarations/methods, but not in async arrow functions?
What's the exact problem here out of interest? Even if there was some grammatical ambiguity with arrow functions, I don't think this would be a major problem as there's already a precedent for some things (this, arguments) not working in arrow functions.
I'm happier with the idea of annotating parameters with await, to indicate that the await happens whether it's a default parameter or not.
Good example, agree with this. Looking through the options, it would certainly be a nice property to always end up with a value, whether a promise or value was passed in, whether via default or not.
FWIW, I would prefer just requiring the explicit await in the function body, for readability/clarity (with a very minor cost in conciseness)
The main issue issue is that entering the function body with a promise for things is not really useful. So you end up entering, awaiting them, and reassigning (or creating another variable) before continuing. Ideally, you want to enter with a value, whether it was originally a promise or not, whether it was the default or not. This was generally a chore that parameter defaults solved, now with async/await not being able to use the same defaulting mechanism becomes more prominent. Plus the fact that you end up with two defaulting solutions (one for promises and one for non-promises) things start to get ugly.
/cc @littledan
pemrouz
commented
Oct 12, 2017
•
What's the exact problem here out of interest? Even if there was some grammatical ambiguity with arrow functions, I don't think this would be a major problem as there's already a precedent for some things (
Good example, agree with this. Looking through the options, it would certainly be a nice property to always end up with a value, whether a promise or value was passed in, whether via default or not.
The main issue issue is that entering the function body with a promise for things is not really useful. So you end up entering, awaiting them, and reassigning (or creating another variable) before continuing. Ideally, you want to enter with a value, whether it was originally a promise or not, whether it was the default or not. This was generally a chore that parameter defaults solved, now with /cc @littledan |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ljharb
Oct 12, 2017
Member
ftr, this, arguments, and super work just fine in arrow functions - the difference is that an arrow function doesn't shadow these bindings with new bindings when it's invoked.
|
ftr, |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
littledan
Oct 12, 2017
Member
Allowing await in non-arrow function parameter defaults seems reasonable. Added this to the November agenda.
To @caitp's concern--wouldn't those users also be confused whenever the default parameter isn't the value that's used?
|
Allowing await in non-arrow function parameter defaults seems reasonable. Added this to the November agenda. To @caitp's concern--wouldn't those users also be confused whenever the default parameter isn't the value that's used? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
caitp
Oct 12, 2017
Contributor
If the default initializer awaits, then you can’t expect the parameter to be a thenable, or else the default case would make no sense..
|
If the default initializer awaits, then you can’t expect the parameter to be a thenable, or else the default case would make no sense.. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
littledan
Oct 12, 2017
Member
@caitp How is this different from all the other cases where the types flowing through JS programs don't necessarily make sense?
|
@caitp How is this different from all the other cases where the types flowing through JS programs don't necessarily make sense? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
caitp
Oct 12, 2017
Contributor
What examples are you thinking of?
I don’t believe this sort of thing has come up before. IMO, if you want a thenable parameter, you shouldn’t await the parameter. If you want a value which could be a thenable which produces a value, you should await it (but the await should be used whether it’s a default parameter or not). There should not be a situation where only the default case awaits.
|
What examples are you thinking of? I don’t believe this sort of thing has come up before. IMO, if you want a thenable parameter, you shouldn’t await the parameter. If you want a value which could be a thenable which produces a value, you should await it (but the await should be used whether it’s a default parameter or not). There should not be a situation where only the default case awaits. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
littledan
Oct 13, 2017
Member
I mean, if I have a function like
function foo(x = 3) {
// use x assuming it's a Number
}isn't this just the same as if I awaited a default and then assumed that the argument is not a thenable?
Anyway, If you're saying that the use case for this is pretty limited, I kind of agree. I can construct cases where it might be useful, though:
function convertCurrency(currencyName, quantity,
conversionRatio = await fetchRatioFromNetwork(currencyName)) {
// ...
}But this example is pretty contrived and I'm not sure that's the clearest way to write code.
@pemrouz Do you have a particular use case you're especially excited about using this for?
|
I mean, if I have a function like function foo(x = 3) {
// use x assuming it's a Number
}isn't this just the same as if I awaited a default and then assumed that the argument is not a thenable? Anyway, If you're saying that the use case for this is pretty limited, I kind of agree. I can construct cases where it might be useful, though: function convertCurrency(currencyName, quantity,
conversionRatio = await fetchRatioFromNetwork(currencyName)) {
// ...
}But this example is pretty contrived and I'm not sure that's the clearest way to write code. @pemrouz Do you have a particular use case you're especially excited about using this for? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
pemrouz
Oct 16, 2017
@littledan, sure: I was thinking as promises become more ubiquitous, being able to conveniently default them would generally make sense on it's own without needing specific examples. In fact, when I personally tried this for the first time, I just expected it would. Actually, even @BrendanEich and @allenwb were surprised too, so I think it's fair to say many people will be confused :P.
In our particular case, firstly I put together a gist on some background on testing. This section from fero might also be useful. Essentially we want to be able to write services as below (the processing itself is obviously made-up):
module.exports = async tradesProcessor({
db = require('./utils/db')
, log = require('utilise/log')('trades-processor')
, mail = require('./utils/mail')
, trades = await fero('trades' , { client: true })
, tenors = await fero('tenors' , { client: true })
, ccypairs = await fero('ccypairs', { client: true })
, somePeople = []
}) {
return trades
.on('change')
.filter(isValidTenor(tenors))
.filter(isValidCCYPair(ccypairs))
.map(db.update('audit'))
.map(mail(somePeople))
.map(execute)
.map(log)
}Here we depend on three other services trades, tenors and ccypairs. Fero clients are just plain objects that represent the current state of that resource, but you can also get a stream of the changes using .on('change') (client/server nodes are the same but clients are read-only, updates are sent to servers to process). The incoming list of new trades is checked against the valid set of tenors and currency pairs at that point in time, and if they are valid, then a database table is updated, a notification is sent to some people, the trade is executed and logged. The above code reads very well. However, let's imagine you can't await parameters. You would have to await and reassign (or create another variable for each):
module.exports = async tradesProcessor({
db = require('./utils/db')
, log = require('utilise/log')('trades-processor')
, mail = require('./utils/mail')
, somePeople = []
, trades
, tenors
, ccypairs
}) {
trades = trades || await fero('trades' , { client: true })
tenors = tenors || await fero('tenors' , { client: true })
ccypairs = ccypairs || await fero('ccypairs', { client: true })
return trades
.on('change')
.filter(isValidTenor(tenors))
.filter(isValidCCYPair(ccypairs))
.map(db.update('audit'))
.map(mail(somePeople))
.map(log)
.map(execute)
}Now unfortunately there's two approaches to defaulting variables. It a lot less easier to read/parse and often have to clarify what's going on. There's more room for error in not properly initialising or defaulting. It's also unnecessary boilerplate. It may not be so bad with a handful of services, but with 100+ small services it's a lot of overhead.
Regarding awaiting the default, or awaiting the parameter, I agree that the latter would be better, however thinking again this isn't a huge deal, since it's mitigated by the fact that you can already easily await before passing something in. Using the same example, our unit tests would look like:
const fakeTenors = ['SPOT', '1W', '1M']
, fakeCCYPairs = { EURUSD: { .. }, GBPUSD: { .. }, USDJPY: { .. } }
tradesProcessor({ .., tenors: fakeTenors, ccyPairs: fakeCCYPairs})i.e. we just inject array/object literals as the tenors and ccypairs. We have the value and we don't even need to pass in a promise.
For integration tests and end-to-end tests, we create real fero servers/clients and inject those.
const servers = {
tenors = await require('./tenors')
, ccypairs = await require('./ccypairs')
}
, clients = {
tenors = await fero('tenors' , { client: true })
, ccypairs = await fero('ccypairs', { client: true })
}
tradesProcessor({ .., tenors: clients.tenors, ccyPairs: clients.ccypairs })i.e. expecting the value to be passed in is completely fine, as you (pretty much always) already have await working on the other side before you pass it in.
pemrouz
commented
Oct 16, 2017
|
@littledan, sure: I was thinking as promises become more ubiquitous, being able to conveniently default them would generally make sense on it's own without needing specific examples. In fact, when I personally tried this for the first time, I just expected it would. Actually, even @BrendanEich and @allenwb were surprised too, so I think it's fair to say many people will be confused :P. In our particular case, firstly I put together a gist on some background on testing. This section from fero might also be useful. Essentially we want to be able to write services as below (the processing itself is obviously made-up): module.exports = async tradesProcessor({
db = require('./utils/db')
, log = require('utilise/log')('trades-processor')
, mail = require('./utils/mail')
, trades = await fero('trades' , { client: true })
, tenors = await fero('tenors' , { client: true })
, ccypairs = await fero('ccypairs', { client: true })
, somePeople = []
}) {
return trades
.on('change')
.filter(isValidTenor(tenors))
.filter(isValidCCYPair(ccypairs))
.map(db.update('audit'))
.map(mail(somePeople))
.map(execute)
.map(log)
}Here we depend on three other services module.exports = async tradesProcessor({
db = require('./utils/db')
, log = require('utilise/log')('trades-processor')
, mail = require('./utils/mail')
, somePeople = []
, trades
, tenors
, ccypairs
}) {
trades = trades || await fero('trades' , { client: true })
tenors = tenors || await fero('tenors' , { client: true })
ccypairs = ccypairs || await fero('ccypairs', { client: true })
return trades
.on('change')
.filter(isValidTenor(tenors))
.filter(isValidCCYPair(ccypairs))
.map(db.update('audit'))
.map(mail(somePeople))
.map(log)
.map(execute)
}Now unfortunately there's two approaches to defaulting variables. It a lot less easier to read/parse and often have to clarify what's going on. There's more room for error in not properly initialising or defaulting. It's also unnecessary boilerplate. It may not be so bad with a handful of services, but with 100+ small services it's a lot of overhead. Regarding awaiting the default, or awaiting the parameter, I agree that the latter would be better, however thinking again this isn't a huge deal, since it's mitigated by the fact that you can already easily const fakeTenors = ['SPOT', '1W', '1M']
, fakeCCYPairs = { EURUSD: { .. }, GBPUSD: { .. }, USDJPY: { .. } }
tradesProcessor({ .., tenors: fakeTenors, ccyPairs: fakeCCYPairs})i.e. we just inject array/object literals as the tenors and ccypairs. We have the value and we don't even need to pass in a promise. For integration tests and end-to-end tests, we create real fero servers/clients and inject those. const servers = {
tenors = await require('./tenors')
, ccypairs = await require('./ccypairs')
}
, clients = {
tenors = await fero('tenors' , { client: true })
, ccypairs = await fero('ccypairs', { client: true })
}
tradesProcessor({ .., tenors: clients.tenors, ccyPairs: clients.ccypairs })i.e. expecting the value to be passed in is completely fine, as you (pretty much always) already have |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
littledan
Oct 16, 2017
Member
This example seems pretty motivating to me. @pemrouz and I took a look at the spec, and it seems like, in spec land, EvaluateBody for async functions somehow needs to be turned around to make FunctionDeclarationInstantiation for async functions come after AsyncFunctionStart, rather than before, as it does now. This might be a little non-trivial. I'm looking forward to spec and test contributions from @pemrouz towards this. If we figure it all out, I've put this on the November agenda to discuss further.
Would it be possible to handle async arrow functions as well? The grammar part seems a bit harder--suddenly, the grammar for async arrow function heads is not a subset of the grammar of CallExpression. Maybe we could recover by making AwaitExpression "always available", and then just banned if it turns out that we're not taking the async arrow function cover grammar? Would this be horrible to handle for parsers in practice? (I'm guessing that this would be fairly horrible, but I'm curious if there are other opinions.)
|
This example seems pretty motivating to me. @pemrouz and I took a look at the spec, and it seems like, in spec land, EvaluateBody for async functions somehow needs to be turned around to make FunctionDeclarationInstantiation for async functions come after AsyncFunctionStart, rather than before, as it does now. This might be a little non-trivial. I'm looking forward to spec and test contributions from @pemrouz towards this. If we figure it all out, I've put this on the November agenda to discuss further. Would it be possible to handle async arrow functions as well? The grammar part seems a bit harder--suddenly, the grammar for async arrow function heads is not a subset of the grammar of CallExpression. Maybe we could recover by making AwaitExpression "always available", and then just banned if it turns out that we're not taking the async arrow function cover grammar? Would this be horrible to handle for parsers in practice? (I'm guessing that this would be fairly horrible, but I'm curious if there are other opinions.) |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ljharb
Oct 16, 2017
Member
I agree, the example is quite compelling.
Assuming it's possible to achieve in the spec, I look forward to discussing it in committee. (I'd also expect async arrows to have the same capabilities as normal async functions)
|
I agree, the example is quite compelling. Assuming it's possible to achieve in the spec, I look forward to discussing it in committee. (I'd also expect async arrows to have the same capabilities as normal async functions) |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
littledan
Oct 16, 2017
Member
Another question to ponder: How bad would it be to have await in parameters available in function declarations, function literals and concise methods, but not async arrow functions? And, should it be available in an async arrow function nested in another async function (which doesn't present parsing problems)?
|
Another question to ponder: How bad would it be to have await in parameters available in function declarations, function literals and concise methods, but not async arrow functions? And, should it be available in an async arrow function nested in another async function (which doesn't present parsing problems)? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ljharb
Oct 16, 2017
Member
I think it would be pretty bad if async function and async () had different capabilities (besides the normal differences for normal vs arrow functions).
|
I think it would be pretty bad if |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
littledan
Oct 16, 2017
Member
@ljharb There's a contradictory principle, though, which is that you can use await everywhere else in async functions, and you can run code in parameter defaults, but you can't run await in async function parameter defaults. Which is worse--the arrow function omission, or the general parameter omission?
|
@ljharb There's a contradictory principle, though, which is that you can use |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ljharb
Oct 16, 2017
Member
That's a tough question. Personally I'd lean towards "all async functions can't use await in defaults" is a better inconsistency than "async arrows can't use something that async functions can".
|
That's a tough question. Personally I'd lean towards "all async functions can't use |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
pemrouz
Oct 16, 2017
Putting the question of whether it might actually be possible aside, I think the principle of "if it doesn't work in one place (arrows), it shouldn't work anywhere" is not a great one, since the change is asymmetric. Arrow functions would remain as-is, but regular functions gain a capability. This would be equivalent to saying: since we don't have arrow generators, no-one can have generators.
Also, as to your question @littledan whether it should work in async arrow functions that are nested in an async function, I just wanted to higlighted the following (not to argue for this, just that there is a precedent for this kind of approach and in case it triggers further ideas/discussion):
Use of the yield keyword
The
yieldkeyword may not be used in an arrow function's body (except when permitted within functions further nested within it). As a consequence, arrow functions cannot be used as generators.
– https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions
pemrouz
commented
Oct 16, 2017
|
Putting the question of whether it might actually be possible aside, I think the principle of "if it doesn't work in one place (arrows), it shouldn't work anywhere" is not a great one, since the change is asymmetric. Arrow functions would remain as-is, but regular functions gain a capability. This would be equivalent to saying: since we don't have arrow generators, no-one can have generators. Also, as to your question @littledan whether it should work in async arrow functions that are nested in an async function, I just wanted to higlighted the following (not to argue for this, just that there is a precedent for this kind of approach and in case it triggers further ideas/discussion):
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ljharb
Oct 17, 2017
Member
@pemrouz tbh, that actually would have been a pretty strong argument against adding generators without arrow generators, and is a strong argument towards adding arrow generators.
The yield example is indeed a precedent, but not one I think we should emulate.
|
@pemrouz tbh, that actually would have been a pretty strong argument against adding generators without arrow generators, and is a strong argument towards adding arrow generators. The |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
littledan
Oct 17, 2017
Member
Another way to look at this is, whichever way you slice it, we have a strange edge case which is somewhat internally motivated. There isn't a great reason for users to have a mental model that parameters can't have await; it's just an arbitrary restriction. Disallowing it in a narrower case might not make it any worse of an internally driven edge case.
|
Another way to look at this is, whichever way you slice it, we have a strange edge case which is somewhat internally motivated. There isn't a great reason for users to have a mental model that parameters can't have |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ljharb
Oct 17, 2017
Member
I think it would make for a more complex mental model than we currently have - I agree that none of the restrictions make sense externally, and ideally we should allow await in defaults in every kind of function.
|
I think it would make for a more complex mental model than we currently have - I agree that none of the restrictions make sense externally, and ideally we should allow |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
anba
Oct 24, 2017
Contributor
Looks like my comment from above needs some further clarifications (I kept it short because we already had similar issues with yield in the past, so I assumed the grammar problem was still on people's radar.)
Given this async arrow async(p = await/a/g) => {};, when we first parse the head async(p = await/a/g) (in an [~Await] context), we lex and parse await/a/g as a division (token sequence: ID(await) DIV ID(a) DIV ID(g)). When we reparse await/a/g during CoveredAsyncArrowHead, we can only reinterpret the originally matched tokens. But reinterpreting the tokens isn't the expected behaviour in this case, because I guess everyone would expect await/a/g to be reparsed as the token sequence AWAIT REGEXP(/a/g).
|
Looks like my comment from above needs some further clarifications (I kept it short because we already had similar issues with Given this async arrow |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ljharb
Oct 24, 2017
Member
Yes, everyone would expect that - how difficult would it be to reparse those tokens as well?
|
Yes, everyone would expect that - how difficult would it be to reparse those tokens as well? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
anba
Oct 24, 2017
Contributor
It's probably only possible to achieve the expected behaviour by reparsing the actual source text instead of reinterpreting the token sequence. And I'm not sure we want to go down that path.
|
It's probably only possible to achieve the expected behaviour by reparsing the actual source text instead of reinterpreting the token sequence. And I'm not sure we want to go down that path. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ljharb
Oct 24, 2017
Member
Could it instead be parsed when it sees await as both options, the first time, and then one discarded when only one possibility remains?
|
Could it instead be parsed when it sees |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
anba
Oct 24, 2017
Contributor
No, I don't think that's possible, because then we'd need to construct two different parse trees in parallel whenever we detect a possible await in a possible async arrow function parameter position.
|
No, I don't think that's possible, because then we'd need to construct two different parse trees in parallel whenever we detect a possible |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
pemrouz
Oct 25, 2017
@anba - thank you for the clarification :)
I'm likely missing something since this seems like an obvious question: but is there any reason it couldn't be parsed as AWAIT REGEXP(/a/g) in the first place? Once you've reached that point, await is not a valid identifier (in either the body or parameters, arrows or non-arrows) anyway and cannot (should not?) be interpreted as one.
Furthermore, looking at these two lines (x = function async( and x = async(), the only difference before the await is the additional function keyword. How does this make a difference between the two cases exactly? Given there is already the async keyword there, why can that not be used to signal any difference in the rules around await that the additional function keyword is doing? Or, why could it not be reparsed when the => is encountered, which should then signal the exact same thing as function?
I'm trying to think of how this might break other cases where await (or async) are a valid identifier, but nothing is coming to mind..
pemrouz
commented
Oct 25, 2017
|
@anba - thank you for the clarification :) I'm likely missing something since this seems like an obvious question: but is there any reason it couldn't be parsed as Furthermore, looking at these two lines ( I'm trying to think of how this might break other cases where |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
anba
Oct 25, 2017
Contributor
When we start parsing async(p = await/a/g), we don't know yet if this is the header of an async arrow function or just a normal call with async and await as identifier names. And for backwards compatibility, we need to assume the identifier case applies to ensure code like this one keeps working:
function async(x) {
console.log(x);
}
var p, await = 10, a = 2, g = 2;
async(p = await/a/g); // Prints 2.5It'd be a totally different story if async and await were proper keywords, like while or function, but for backwards compatibility we had to make them contextual keywords.
Furthermore, looking at these two lines (
x = function async(andx = async(), the only difference before theawaitis the additionalfunctionkeyword. How does this make a difference between the two cases exactly?
When we see async function, we directly know that the current source code starts an async function declaration (or async function expression), but if we only have async(, we need to parse until the => to know if this is a call expression or an async arrow function. And because unbound lookahead or backtracking isn't allowed, we need to treat async( as the start of a call expression when we first parse it.
Given there is already the
asynckeyword there, why can that not be used to signal any difference in the rules aroundawaitthat the additionalfunctionkeyword is doing?
Except async isn't a proper keyword, we only interpret it as a keyword depending on the context.
Or, why could it not be reparsed when the
=>is encountered, which should then signal the exact same thing asfunction?
Currently the spec defines "reparsing" as reinterpreting an already parsed sequence of tokens, so for that kind of "reparsing" it's already too late to change await/a/g from a division expression to an await expression. So we'd need to "reparse" the actual source text to get the expected await expression. Which means we'd effectively need to disallow any kind of streaming parser, because the source text needs to be kept in memory in case we need to rewind it for an async arrow function. And there are probably also other issues (implementation- and specification-wise), but the streaming case comes first to my mind.
|
When we start parsing function async(x) {
console.log(x);
}
var p, await = 10, a = 2, g = 2;
async(p = await/a/g); // Prints 2.5It'd be a totally different story if
When we see
Except
Currently the spec defines "reparsing" as reinterpreting an already parsed sequence of tokens, so for that kind of "reparsing" it's already too late to change |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
pemrouz
Oct 25, 2017
@anba, thanks again. In that case, there seems to be a few options (or just ideas rather). To begin with: when we reparse CoverCallExpressionAndAsyncArrowHead as an AsyncArrowHead during CoveredAsyncArrowHead, why can't those tokens be reparsed then? To be clear, I think this is @ljharb's first question, to which you reply reparsing the sourcing text is probably the only possible solution, but "I'm not sure we want to go down that path". Would you be able to elaborate on why not?
pemrouz
commented
Oct 25, 2017
|
@anba, thanks again. In that case, there seems to be a few options (or just ideas rather). To begin with: when we reparse CoverCallExpressionAndAsyncArrowHead as an AsyncArrowHead during CoveredAsyncArrowHead, why can't those tokens be reparsed then? To be clear, I think this is @ljharb's first question, to which you reply reparsing the sourcing text is probably the only possible solution, but "I'm not sure we want to go down that path". Would you be able to elaborate on why not? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
anba
Oct 26, 2017
Contributor
when we reparse CoverCallExpressionAndAsyncArrowHead as an AsyncArrowHead during CoveredAsyncArrowHead, why can't those tokens be reparsed then?
Do you mean why we can't take the token sequence ID(await) DIV ID(a) DIV ID(g) from await/a/g and somehow convert it into AWAIT REGEXP(/a/g)? That's because of white space: Tokens don't have any white space information, because white space between tokens is, in general, irrelevant. But within a regular expression, white space is significant.
[...] to which you reply reparsing the sourcing text is probably the only possible solution, but "I'm not sure we want to go down that path". Would you be able to elaborate on why not?
For example because this will make it impossible (or at least more difficult) to create a streaming parser. And reparsing the source text is also not nice for time complexity reasons. If you run this example program, you can easily tell which implementations reparse the source text (*) and which ones only reparse/reinterpret the token sequence:
for (var i = 1; i <= 20; ++i) {
var s = "(a = ".repeat(i) + "0" + ")=>{}".repeat(i);
var t = Date.now();
eval(s);
print(Date.now() - t);
}(*) Hmm, mentioning this could a red herring, but some implementations are currently reparsing the source text instead of reparsing the token sequence, because that's easier to implement, but I really don't think we should require this behaviour in the spec itself.
Do you mean why we can't take the token sequence
For example because this will make it impossible (or at least more difficult) to create a streaming parser. And reparsing the source text is also not nice for time complexity reasons. If you run this example program, you can easily tell which implementations reparse the source text (*) and which ones only reparse/reinterpret the token sequence: for (var i = 1; i <= 20; ++i) {
var s = "(a = ".repeat(i) + "0" + ")=>{}".repeat(i);
var t = Date.now();
eval(s);
print(Date.now() - t);
}(*) Hmm, mentioning this could a red herring, but some implementations are currently reparsing the source text instead of reparsing the token sequence, because that's easier to implement, but I really don't think we should require this behaviour in the spec itself. |
pushed a commit
to pemrouz/ecma262
that referenced
this issue
Mar 18, 2018
pushed a commit
to pemrouz/ecma262
that referenced
this issue
Mar 18, 2018
pemrouz
referenced this issue
Mar 18, 2018
Open
normative: allow await in parameter defaults (#917) #1136
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
pemrouz
Mar 20, 2018
After discussion in committee, most people generally seemed to lean towards the inconsistency of await not working in default parameters, than the inconsistency of it not working in arrow functions. At the same time, I don't really have that strong preference either way and there wasn't any great ideas for how to solve it for arrows. Hence, unless someone else wants to come up with a suggestion on how to fix the grammar issues, just wanted to close this issue out now. Thanks all for your feedback/comments/help on this!
pemrouz
commented
Mar 20, 2018
|
After discussion in committee, most people generally seemed to lean towards the inconsistency of await not working in default parameters, than the inconsistency of it not working in arrow functions. At the same time, I don't really have that strong preference either way and there wasn't any great ideas for how to solve it for arrows. Hence, unless someone else wants to come up with a suggestion on how to fix the grammar issues, just wanted to close this issue out now. Thanks all for your feedback/comments/help on this! |
pemrouz
closed this
Mar 20, 2018
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
WebReflection
Mar 20, 2018
When we start parsing
async(p = await/a/g), we don't know yet if this is the header of an async arrow function or just a normal call withasyncandawaitas identifier names.
This is something I've never fully understood.
In order to validate any function invoke you need to eventually wait 'till the end of the scope to see of there is a function declaration.
This, unless you already know there is a function named async in the current scope, before parsing that part, or in any outer scope previously parsed ... right?
Accordingly, if function declarations are possible in JavaScript parsing, what is the issue in waiting for the eventual arrow async(...) =>, before deciding any sort for an async name in the wild?
WebReflection
commented
Mar 20, 2018
•
This is something I've never fully understood. In order to validate any function invoke you need to eventually wait 'till the end of the scope to see of there is a function declaration. This, unless you already know there is a function named Accordingly, if function declarations are possible in JavaScript parsing, what is the issue in waiting for the eventual arrow |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
WebReflection
Mar 20, 2018
beside me liking the idea, I've found your examples both broken (syntax speaking) and not too compelling.
It's very easy to already simplify your use case, either via arguments
async function withAsyncParams() {
let [
a = 1,
b = await Promise.resolve(2)
] = arguments;
return a + b;
}
withAsyncParams().then(console.log); // 3or via default destructuring
async function withAsyncDestructuring($ = {}) {
const {
a = 1,
b = await Promise.resolve(2)
} = $;
return a + b;
}
withAsyncDestructuring().then(console.log); // 3Accordingly, your example could already work either like this:
module.exports = async function tradesProcessor() {
let [
db = require('./utils/db'),
log = require('utilise/log')('trades-processor'),
mail = require('./utils/mail'),
trades = await fero('trades' , { client: true }),
tenors = await fero('tenors' , { client: true }),
ccypairs = await fero('ccypairs', { client: true }),
somePeople = []
] = arguments;
return trades
.on('change')
.filter(isValidTenor(tenors))
.filter(isValidCCYPair(ccypairs))
.map(db.update('audit'))
.map(mail(somePeople))
.map(execute)
.map(log);
};or this
module.exports = async function tradesProcessor($ = {}) {
const {
db = require('./utils/db'),
log = require('utilise/log')('trades-processor'),
mail = require('./utils/mail'),
trades = await fero('trades' , { client: true }),
tenors = await fero('tenors' , { client: true }),
ccypairs = await fero('ccypairs', { client: true }),
somePeople = []
} = $;
return trades
.on('change')
.filter(isValidTenor(tenors))
.filter(isValidCCYPair(ccypairs))
.map(db.update('audit'))
.map(mail(somePeople))
.map(execute)
.map(log);
};right?
WebReflection
commented
Mar 20, 2018
•
|
beside me liking the idea, I've found your examples both broken (syntax speaking) and not too compelling. It's very easy to already simplify your use case, either via async function withAsyncParams() {
let [
a = 1,
b = await Promise.resolve(2)
] = arguments;
return a + b;
}
withAsyncParams().then(console.log); // 3or via default destructuring async function withAsyncDestructuring($ = {}) {
const {
a = 1,
b = await Promise.resolve(2)
} = $;
return a + b;
}
withAsyncDestructuring().then(console.log); // 3Accordingly, your example could already work either like this: module.exports = async function tradesProcessor() {
let [
db = require('./utils/db'),
log = require('utilise/log')('trades-processor'),
mail = require('./utils/mail'),
trades = await fero('trades' , { client: true }),
tenors = await fero('tenors' , { client: true }),
ccypairs = await fero('ccypairs', { client: true }),
somePeople = []
] = arguments;
return trades
.on('change')
.filter(isValidTenor(tenors))
.filter(isValidCCYPair(ccypairs))
.map(db.update('audit'))
.map(mail(somePeople))
.map(execute)
.map(log);
};or this module.exports = async function tradesProcessor($ = {}) {
const {
db = require('./utils/db'),
log = require('utilise/log')('trades-processor'),
mail = require('./utils/mail'),
trades = await fero('trades' , { client: true }),
tenors = await fero('tenors' , { client: true }),
ccypairs = await fero('ccypairs', { client: true }),
somePeople = []
} = $;
return trades
.on('change')
.filter(isValidTenor(tenors))
.filter(isValidCCYPair(ccypairs))
.map(db.update('audit'))
.map(mail(somePeople))
.map(execute)
.map(log);
};right? |

pemrouz commentedMay 14, 2017
Following on from this thread, just creating this issue to help nudge discussion forward. Based on various comments, it seems doable but requires spec changes first.
Clearly the following lines need to be removed, but how exactly the parsing logic needs to change to accommodate this is less clear to me:
Potential syntax variations for discussion:
await just default value
await param, not just default value:
await param, with default value