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

n-api: Use get_all_property_names for Object::get_own_property_names impl #511

Closed
goto-bus-stop opened this issue Apr 24, 2020 · 1 comment

Comments

@goto-bus-stop
Copy link
Member

Object::get_own_property_names should act like Object.getOwnPropertyNames does in JS, but there was no n-api function available to do exactly that. Instead, we're using napi_get_property_names, which acts more like for (var key in object), including properties from the prototype and the like.

Right now, we're doing this load of work to paper over that: https://github.com/goto-bus-stop/neon/blob/9b338f9ed0a8dcb781566b61bc06b67f07ca23a1/crates/neon-runtime/src/napi/object.rs#L17-L84

But it's still not accurate.

There is an napi_get_all_property_names API that would allow implementing Object::get_own_property_names correctly, and with a lot less code to boot. At the time of the PR, it was still in the process of being backported. Once it is available in all release lines (Node.js 10/12/13/14), we can switch to napi_get_all_property_names.

@goto-bus-stop
Copy link
Member Author

Done in #601!

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

1 participant