Conversation
This just ensures that the superFunction is wrapped with a terminal _super instead of relying on this.__nextSuper = null while invoked to prevent recursion. Allows wrapped functions not to have to go through 2 argument applies, makes debugging a lot easier and performs better.
|
Seems good to me. @stefanpenner / @mixonic - Y'all would probably be better reviewers here... |
There was a problem hiding this comment.
__nextSuper is gone, but __hasSuper will be present and enumerable when there is a _super call in a function correct?
There was a problem hiding this comment.
The paranoid me wants to suggest leaving __nextSuper and the enumerable: false and using that instead of a new property. I know you probably want to drop the defineProperty, but it does not seem to be a change core to what you've done in this PR.
There was a problem hiding this comment.
@rwjblue reviews my code review and find it's lack of attention disturbing.
He points out __hasSuper is on the function. Inception level 3 achieved! Love it.
There was a problem hiding this comment.
That code was just moved up from mixin
|
Very cool. |
|
Nice, this is more similar to what I did in core-object. The only thing is the super call with the apply arguments likely needs to build the array as it did before. Do to a bug in v8, although it should be fixed for desktop versions, I believe it is still common on most mobile variants |
|
we should get this in, @krisselden feels confident this doesn't regress the deopt. If it does we can fix, this ultimately the right direction. |
4 participants
This just ensures that the superFunction is wrapped with a terminal _super instead of relying on this.__nextSuper = null while invoked to prevent recursion. Allows wrapped functions not to have to go through 2 argument applies, makes debugging a lot easier and performs better.