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

Consider "new.target" for constructor callbacks #200

Closed
jasongin opened this issue Mar 26, 2017 · 7 comments
Closed

Consider "new.target" for constructor callbacks #200

jasongin opened this issue Mar 26, 2017 · 7 comments
Assignees

Comments

@jasongin
Copy link
Member

Reference nodejs/node#11975 (comment), nodejs/node#9293, nodejs/node#10915

In mixed JS/native inheritance scenarios, it can be necessary to get the "new target" for the constructor, via v8::FunctionCallbackInfo<T>::NewTarget(). In N-API this would be exposed as napi_get_cb_new_target() or something similar.

@boingoing boingoing changed the title Consider "new target" for constructor callbacks Consider "new.target" for constructor callbacks Mar 27, 2017
@boingoing
Copy link

Should this be added as an optional out parameter to napi_get_cb_info? If we remove the other methods in this family since one method could handle all of their use cases, it seems to make sense. Would make sense to do that together with #196.

@jasongin
Copy link
Member Author

@boingoing I don't think it should be added to napi_get_cb_info() because it would be very rarely needed, only by some constructors. Maybe it could be combined with napi_is_construct_call() into a napi_get_cb_construct_info() or something like that.

@boingoing
Copy link

Maybe it could be combined with napi_is_construct_call() into a napi_get_cb_construct_info() or something like that.

This sounds like a good plan to me. Only constructors would be interested in this flag or new.target generally.

@jasongin
Copy link
Member Author

jasongin commented Aug 1, 2017

Actually a callback is a constructor call if and only if a new target is defined. So I think we can just change napi_is_construct_call() to napi_get_new_target().

@mhdawson
Copy link
Member

mhdawson commented Aug 3, 2017

That makes sense to me.

@sampsongao
Copy link
Collaborator

nodejs/node#14698

@mhdawson
Copy link
Member

Looks like related PR landed, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants