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

Migration from NAN to object_wrap / determine invoked property name #1114

Open
pdehne opened this issue Dec 23, 2021 · 15 comments
Open

Migration from NAN to object_wrap / determine invoked property name #1114

pdehne opened this issue Dec 23, 2021 · 15 comments

Comments

@pdehne
Copy link

pdehne commented Dec 23, 2021

This is a usage / migration from NAN question. With NAN I used the following to determine the invoked property name:

NAN_GETTER(ClassName::Getters)
{
        ...        
        std::string propertyName = *Nan::Utf8String(property);
        ...
}

This helped me to reduce C++ boilerplate code for classes with many properties because I did not have to declare and define every single setter and getter method in C++.

Is this still possible?

@github-actions
Copy link

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@mhdawson
Copy link
Member

Added to list of issues to discussion in next node-api team meeting

@mhdawson
Copy link
Member

@vmoroz will take a look this week.

@mhdawson
Copy link
Member

mhdawson commented Apr 8, 2022

We talked about this is in the Node-API team meeting today and we don't have any easy way to do this today.

@vmoroz will think a bit more about it.

@vmoroz
Copy link
Member

vmoroz commented Apr 15, 2022

My understanding of the issue is that NAN uses the V8 ObjectTemplate that uses Proxy-like methods for Object's getters and setters to access all object properties. In Node-API we use a different approach: we define a JavaScript class as a FunctionTemplate and then explicitly add instance and static properties.
I would suggest that we add to Node-API a new construct to define a native object that could use the V8 ObjectTemplate and Proxy for other JavaScript engines.
Until this new API is added, developers can achieve the same effect by using existing Node-API to create a Proxy object.

@KevinEady
Copy link
Contributor

In today's Node API meeting, we discussed the possibility of using the Proxy object as a wrapper on the natively ObjectWrap'd object. Using this approach, we may be able to create the "dynamicness" that is requested in this original post, for example passing a get handler that would pass the property name to a native method provided by the underlying ObjectWrap'd object.

@vmoroz will take a look and attempt to create some unit tests that does this and provide feedback, determining if we need to enhance the Node API for anything.

@vmoroz
Copy link
Member

vmoroz commented Apr 22, 2022

We used a very similar approach before to implement JSI on top of Node-API.
The plan is to add unit tests to depo the approach with the plain Node-API and the C++ Node-API.
Then, we will see if we can add C++ Node-API extensions to help migrating from NAN.

@vmoroz
Copy link
Member

vmoroz commented Apr 29, 2022

I had started to work on the unit test.
So far, it works for a very simple object. I am going to continue to improve the code to cover more scenarios.
nodejs/node#42911

@github-actions
Copy link

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@mhdawson
Copy link
Member

Just leaving open, as @vmoroz is planning to add a C++ version of the example. The C based one he added has landed.

@github-actions
Copy link

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions github-actions bot added the stale label Dec 23, 2022
@mhdawson mhdawson removed the stale label Jan 3, 2023
@github-actions
Copy link

github-actions bot commented Apr 4, 2023

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions github-actions bot added the stale label Apr 4, 2023
@mhdawson mhdawson removed the stale label Apr 4, 2023
@github-actions
Copy link

github-actions bot commented Jul 4, 2023

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions
Copy link

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions github-actions bot added the stale label Oct 11, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 11, 2023
@vmoroz vmoroz removed the stale label Nov 11, 2023
@vmoroz
Copy link
Member

vmoroz commented Nov 11, 2023

Reopening the issue. We still want to find a good solution here.

@vmoroz vmoroz reopened this Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants