Skip to content
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

Pass instance into oncreate as argument, or .call it #557

Closed
Rich-Harris opened this issue May 3, 2017 · 2 comments
Closed

Pass instance into oncreate as argument, or .call it #557

Rich-Harris opened this issue May 3, 2017 · 2 comments
Labels
awaiting submitter needs a reproduction, or clarification

Comments

@Rich-Harris
Copy link
Member

Follow-up to #525 (comment).bind is slow and we should avoid it.

The code in question is here:

options._root._renderHooks.push( template.oncreate.bind( this ) );

We have a few options:

Go back to passing the context alongside the function

options._root._renderHooks.push({ fn: template.oncreate, context: this });

Not totally ideal, since a) you might not have a context, and b) if we were to include additional arguments (#550) then you'd have to deal with those too.

Wrap the function with a .call

var self = this;
options._root._renderHooks.push( function () {
  template.oncreate.call( self );
});

Rewrite the function so that it takes the instance as an argument

var self = this;
options._root._renderHooks.push( function () {
  template.oncreate( self );
});

I poked around a bit the other night and I don't actually think that's any faster than .call, so probably not worth it.

I'm also not certain that wrapping the function is quicker than .bind, we'd have to check. If not, then maybe we do need to consider going back to the old way of doing things.

@Conduitry
Copy link
Member

I think this is already taken care of now, perhaps by the recent lifecycle hook changes? oncreate is now getting called with oncreate.call(self);

@arxpoetica arxpoetica added the awaiting submitter needs a reproduction, or clarification label Apr 20, 2018
@Conduitry
Copy link
Member

Gonna go ahead and close this - I'm pretty sure this has been taken care of for a while now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification
Projects
None yet
Development

No branches or pull requests

3 participants