Default derived constructor uses Rest and Spread argument #7678
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/7480/ |
|
|
||
| let params, body; | ||
|
|
||
| if (classState.isDerived) { | ||
| const constructor = template.expression.ast` | ||
| (function () { | ||
| super(...arguments); | ||
| (function (...args) { |
There was a problem hiding this comment.
An issue description would be helpful. What's the motivation for this change?
There was a problem hiding this comment.
I think that in loose mode we should keep using arguments, since it produces a smaller (and probably faster) output.
| @@ -349,20 +346,9 @@ export default function transformClass( | |||
| call = t.logicalExpression("||", bareSuperNode, t.thisExpression()); | |||
| } else { | |||
| bareSuperNode = optimiseCall( | |||
There was a problem hiding this comment.
Looks like the helper-optimise-call-expression needs to be updated to support the super(...args) case; right now the optimizeCall is essentially a noop (or, well, it makes a .call, but not an "optimized" one).
Alternatively, if this is a special case, we can't use the helper and have to do the optimization locally.
| function BaseController2() { | ||
| return _Chaplin$Controller$A.apply(this, arguments) || this; | ||
| function BaseController2(...args) { | ||
| return _Chaplin$Controller$A.call.apply(_Chaplin$Controller$A, [this].concat(args)) || this; |
There was a problem hiding this comment.
I wonder how this might affect class instantiation performance? My main concern with this change is that it increases output complexity for an extremely edge-casey issue that doesn't seem like anyone would rely on.
There was a problem hiding this comment.
I'm working on a total refactor here, it's just really difficult because the code's a bit low quality at the moment. This will be optimized into a straight #apply when I'm done.
There was a problem hiding this comment.
See #7678 (comment); helper-optimise-call-expression just doesn't deal with this corner case and is inserting a .call instead of an .apply
|
Closing since tc39/ecma262#2216 got consensus during the last TC39 meeting. |
Fixes #5147.
Depends on #7675.