-
Notifications
You must be signed in to change notification settings - Fork 27
AggregateError argument order seems backwards #44
Comments
This was discussed in #14. |
Let's make |
That was also discussed (and rejected, at least by e.g. me) in #14. Fwiw I would also find it more surprising to have |
Neither seems optional. But if we have to have "optional" things first, then making them both optional lets us use the least surprising ordering. |
I don't share this intuition at all. I encounter
Again, I would find your proposed ordering more surprising. But also, even if this were not the case, all of
seem like more important principles, and collectively imply the current ordering. Incidentally, the current ordering also matches other error types (OverconstrainedError, RTCError) in the web platform. (The latter of those at my request, but it's shipping now.) |
Currently it's not possible to switch order of arguments for |
I still think this is ridiculous. If we want class AggregateError {
constructor(messageOrErrors, errors) {
if (typeof messageOrErrors !== 'string') {
errors = messageOrErrors;
messageOrErrors = '';
}
// ...
}
} |
While i think the arg order should be |
I think you're using it much more than you realize. Just doing a doc search, I can find: Node:
Express:
For node, this is extremely common, because the overriding pattern is "callback goes last". If you want optional parameters, they go before that callback. I think this can be an excellent API pattern, and anything else would have felt worse. (I can find more, but they're the easiest ones to find are always Node ones because they follow this pattern).
I don't think we've had the need in 262 yet, so there aren't any. I can't really think of any API that would have been made better by this, until now.
As in tc39/proposal-iterator.range#18, I think this an extremely reasonable pattern to use. |
node’s and express’ patterns were born and largely locked in by 2012, long before that became a widely accepted antipattern. It’s because i use it a lot that i understand why it’s suboptimal. |
Not that it justifies anything here, but the Function constructors interpret their arguments as a list of parameter names followed by a body (and the Array constructor arguably also juggles for |
Why is
errors
the first parameter? Shouldn'tmessage
be first?If someone were to switch from a
TypeError
to anAggregateError
, it'd be surprising thatmessage
is now second.As a bonus, if there are multiple errors, it's pleasing that the array can be multiple lines without a trailing message argument. Kinda like how node's
callback
is always last:The text was updated successfully, but these errors were encountered: