Add API for deletion of object properties #151
Conversation
| /// Delete property. | ||
| bool Delete( | ||
| Value key ///< Property key | ||
| ); |
There was a problem hiding this comment.
I don’t think we need this key since Value has an operator napi_value member, right?
There was a problem hiding this comment.
Indeed I guess so; I just copied the definitions from Object::Has. Should I remove all of them?
There was a problem hiding this comment.
I think I had to add the overloads that take Value in order to avoid compiler errors in some situations about ambiguous calls to overloaded methods.
There was a problem hiding this comment.
@jasongin I think it should be safe to remove those – if you’re talking about #86, that’s happening because there were two multiple arguments that were used for overloading …
@rolftimmermans I’m not sure, but it certainly doesn’t have to happen in this PR :)
There was a problem hiding this comment.
Oh, wait, maybe that is about Strings that also have an operator std::string? That would make sense but we should have tests for that :)
|
@jasongin Done – I couldn't find any tests though, is that correct? |
|
@rolftimmermans Test coverage is not great right now but you are welcome to contribute to improving it! |
|
@rolftimmermans the tests are in https://github.com/nodejs/node-addon-api/tree/master/test and you can run them with At some point we should require tests for any new APIs' but as @jasongin mentions we don't have great coverage yet so at this point it would be nice but not required. |
PR-URL: #151 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
|
Landed as b1c4cb9 |
PR-URL: nodejs/node-addon-api#151 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: nodejs/node-addon-api#151 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: nodejs/node-addon-api#151 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: nodejs/node-addon-api#151 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
An API that uses
napi_delete_propertywas missing.This adds a few overloaded
Object::Delete()methods that remove a given property.