Add support for private stuff

[gabrielschulhof]

We should expose the engine's ability of assigning any number of private properties to an object.

When an engine doesn't support this directly, we can implement it by assigning a plain object using napi_wrap() and casting string-valued napi_value items as napi_private and setting properties on the wrapped object and implementing napi_wrap() as the (if necessary) creation of this object and assignment to a well-known private property.

The benefit for the case where the engine does not support private natively is that if private is used via N-API we get type-checking by distinguishing in the API between napi_private and napi_value even though in the implementation there is no distinction.

Contrariwise, when an engine does support private, we can implement napi_wrap() as an assignment to a well-known private property - the key for which we can cache in the environment. So, in both cases we re-implement napi_wrap() only when the engine supports private, we do not need the engine's ability to associate a native pointer with the plain object.

Here's my proposed API so far:

// node_api_types.h
...
typedef struct napi_private__ *napi_private;
...

// node_api.h
...
// Methods to deal with private keys (we document that this API is idempotent)
NAPI_EXTERN napi_status napi_get_private_for_key(napi_env env,
                                                 const char* key,
                                                 napi_private* result);
...
// Methods to work with Objects
...
NAPI_EXTERN napi_status napi_set_private_property(napi_env env,
                                                  napi_value object,
                                                  napi_private key,
                                                  napi_value value);
NAPI_EXTERN napi_status napi_has_private_property(napi_env env,
                                                  napi_value object,
                                                  napi_private key,
                                                  bool* result);
NAPI_EXTERN napi_status napi_get_private_property(napi_env env,
                                                  napi_value object,
                                                  napi_private key,
                                                  napi_value* result);
NAPI_EXTERN napi_status napi_delete_private_property(napi_env env,
                                                     napi_value object,
                                                     napi_private key,
                                                     bool* result);
...

[gabrielschulhof]

Re nodejs/node#14367

[jasongin]

Do we have evidence that this functionality is actually needed by any native modules? None of the ones we have looked at so far have needed it.

Especially since it is somewhat V8-specific, I think we need a good justification before exposing this as part of N-API.

[gabrielschulhof]

@jasongin when adding a native pointer required that the object be special (i.e. created from an object template that had room for native pointers) then there was perhaps less of a chance that there would be a collision. Though even then it was possible to simply have one field pointer kicked out with extreme prejudice, to be replaced by another. Now, we claim that such a pointer can be added to any object. Thus, for the sake of modularity it would be prudent that we allow attached native pointers to coexist, rather than forcing them to collide.

In terms of existing cases where this could be used, consider that @sampsongao is running into the problem where a napi_wrap()ed object needs to have its wrapping undone and then re-done. To that end the napi_(get|set|has|delete)_private_property is perhaps the better abstraction.

[gabrielschulhof]

TBH we could get rid of napi_wrap() entirely.

[gabrielschulhof]

OTOH, and after talking to @mhdawson, perhaps we should restrict ourselves to providing a napi_remove_wrap() so that @sampsongao can use that for node-sass.

[mhdawson]

My preference is that we add functionality where we have at least one consumer that needs it. The larger the API surface the greater the more to support and the more risk that we run into a "breaking" problem in the future.

So my suggestion would be that we defer this for now and stick with providing napi_remove_wrap() which I see is already in progress.

[digitalinfinity]

@gabrielschulhof are you ok to close this until we find a module that shows the need for this feature?

[gabrielschulhof]

Absolutely.