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

node-api: add node_api_symbol_for() #41329

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2492,6 +2492,38 @@ This API creates a JavaScript `symbol` value from a UTF8-encoded C string.
The JavaScript `symbol` type is described in [Section 19.4][]
of the ECMAScript Language Specification.

#### `node_api_symbol_for`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we maybe alias all functions as node_api_* for consistency? We probably cannot rename the symbols since we don't want to break binary compatibility, but mixing the old and new names here is surprising. Maybe we can simply #define new names for all old functions and then use the new names in the documentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good idea to me. Will do this in a separate PR if there are no objections. 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we had discussed doing that but if I remember correctly that could like to add-ons not compiling with earlier versions of Node.js. Either way +1 to a different PR just wanted to mention that we might not be able to do it as easily as we'd like.


<!-- YAML
added: REPLACEME
napiVersion: REPLACEME
-->

> Stability: 1 - Experimental

```c
napi_status node_api_symbol_for(napi_env env,
const char* utf8description,
size_t length,
napi_value* result)
```

* `[in] env`: The environment that the API is invoked under.
* `[in] utf8description`: UTF-8 C string representing the text to be used as the
description for the symbol.
* `[in] length`: The length of the description string in bytes, or
`NAPI_AUTO_LENGTH` if it is null-terminated.
* `[out] result`: A `napi_value` representing a JavaScript `symbol`.

Returns `napi_ok` if the API succeeded.

This API searches in the global registry for an existing symbol with the given
description. If the symbol already exists it will be returned, otherwise a new
symbol will be created in the registry.

The JavaScript `symbol` type is described in [Section 19.4][] of the ECMAScript
Language Specification.

#### `napi_create_typedarray`

<!-- YAML
Expand Down
6 changes: 6 additions & 0 deletions src/js_native_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ NAPI_EXTERN napi_status napi_create_string_utf16(napi_env env,
NAPI_EXTERN napi_status napi_create_symbol(napi_env env,
napi_value description,
napi_value* result);
#ifdef NAPI_EXPERIMENTAL
NAPI_EXTERN napi_status node_api_symbol_for(napi_env env,
const char* utf8description,
size_t length,
napi_value* result);
#endif // NAPI_EXPERIMENTAL
NAPI_EXTERN napi_status napi_create_function(napi_env env,
const char* utf8name,
size_t length,
Expand Down
21 changes: 21 additions & 0 deletions src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1662,6 +1662,27 @@ napi_status napi_create_symbol(napi_env env,
return napi_clear_last_error(env);
}

napi_status node_api_symbol_for(napi_env env,
const char* utf8description,
size_t length,
napi_value* result) {
CHECK_ENV(env);
CHECK_ARG(env, result);

napi_value js_description_string;
STATUS_CALL(napi_create_string_utf8(env,
utf8description,
length,
&js_description_string));
v8::Local<v8::String> description_string =
v8impl::V8LocalValueFromJsValue(js_description_string).As<v8::String>();

*result = v8impl::JsValueFromV8LocalValue(
v8::Symbol::For(env->isolate, description_string));

return napi_clear_last_error(env);
}

static inline napi_status set_error_code(napi_env env,
v8::Local<v8::Value> error,
napi_value code,
Expand Down
11 changes: 5 additions & 6 deletions test/js-native-api/test_properties/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,11 @@ assert.ok(!propertyNames.includes('readwriteAccessor2'));
assert.ok(!propertyNames.includes('readonlyAccessor1'));
assert.ok(!propertyNames.includes('readonlyAccessor2'));

// Validate property created with symbol
const start = 'Symbol('.length;
const end = start + 'NameKeySymbol'.length;
const symbolDescription =
String(Object.getOwnPropertySymbols(test_object)[0]).slice(start, end);
assert.strictEqual(symbolDescription, 'NameKeySymbol');
// Validate properties created with symbol
const propertySymbols = Object.getOwnPropertySymbols(test_object);
assert.strictEqual(propertySymbols[0].toString(), 'Symbol(NameKeySymbol)');
assert.strictEqual(propertySymbols[1].toString(), 'Symbol()');
assert.strictEqual(propertySymbols[2], Symbol.for('NameKeySymbolFor'));

// The napi_writable attribute should be ignored for accessors.
const readwriteAccessor1Descriptor =
Expand Down
13 changes: 13 additions & 0 deletions test/js-native-api/test_properties/test_properties.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#define NAPI_EXPERIMENTAL
#include <js_native_api.h>
#include "../common.h"

Expand Down Expand Up @@ -77,13 +78,25 @@ napi_value Init(napi_env env, napi_value exports) {
NODE_API_CALL(env,
napi_create_symbol(env, symbol_description, &name_symbol));

napi_value name_symbol_descriptionless;
NODE_API_CALL(env,
napi_create_symbol(env, NULL, &name_symbol_descriptionless));

napi_value name_symbol_for;
NODE_API_CALL(env, node_api_symbol_for(env,
"NameKeySymbolFor",
NAPI_AUTO_LENGTH,
&name_symbol_for));

napi_property_descriptor properties[] = {
{ "echo", 0, Echo, 0, 0, 0, napi_enumerable, 0 },
{ "readwriteValue", 0, 0, 0, 0, number, napi_enumerable | napi_writable, 0 },
{ "readonlyValue", 0, 0, 0, 0, number, napi_enumerable, 0},
{ "hiddenValue", 0, 0, 0, 0, number, napi_default, 0},
{ NULL, name_value, 0, 0, 0, number, napi_enumerable, 0},
{ NULL, name_symbol, 0, 0, 0, number, napi_enumerable, 0},
{ NULL, name_symbol_descriptionless, 0, 0, 0, number, napi_enumerable, 0},
{ NULL, name_symbol_for, 0, 0, 0, number, napi_enumerable, 0},
{ "readwriteAccessor1", 0, 0, GetValue, SetValue, 0, napi_default, 0},
{ "readwriteAccessor2", 0, 0, GetValue, SetValue, 0, napi_writable, 0},
{ "readonlyAccessor1", 0, 0, GetValue, NULL, 0, napi_default, 0},
Expand Down
19 changes: 19 additions & 0 deletions test/js-native-api/test_reference/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,25 @@ async function runTests() {
})();
test_reference.deleteReference();

(() => {
const symbol = test_reference.createSymbolFor('testSymFor');
test_reference.createReference(symbol, 0);
assert.strictEqual(test_reference.referenceValue, symbol);
assert.strictEqual(test_reference.referenceValue, Symbol.for('testSymFor'));
})();
test_reference.deleteReference();

(() => {
const symbol = test_reference.createSymbolForEmptyString();
test_reference.createReference(symbol, 0);
assert.strictEqual(test_reference.referenceValue, symbol);
assert.strictEqual(test_reference.referenceValue, Symbol.for(''));
})();
test_reference.deleteReference();

assert.throws(() => test_reference.createSymbolForIncorrectLength(),
/Invalid argument/);

(() => {
const value = test_reference.createExternal();
assert.strictEqual(test_reference.finalizeCount, 0);
Expand Down
39 changes: 39 additions & 0 deletions test/js-native-api/test_reference/test_reference.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#define NAPI_EXPERIMENTAL
#include <stdlib.h>
#include <assert.h>
#include <js_native_api.h>
Expand Down Expand Up @@ -49,6 +50,41 @@ static napi_value CreateSymbol(napi_env env, napi_callback_info info) {
return result_symbol;
}

static napi_value CreateSymbolFor(napi_env env, napi_callback_info info) {

size_t argc = 1;
napi_value args[1];

char description[256];
size_t description_length;

NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL,NULL));
NODE_API_ASSERT(env, argc == 1, "Expect one argument only (symbol description)");

NODE_API_CALL(env, napi_get_value_string_utf8(env, args[0], description, sizeof(description), &description_length));
NODE_API_ASSERT(env, description_length <= 255, "Cannot accommodate descriptions longer than 255 bytes");

napi_value result_symbol;

NODE_API_CALL(env, node_api_symbol_for(env,
description,
description_length,
&result_symbol));
return result_symbol;
}

static napi_value CreateSymbolForEmptyString(napi_env env, napi_callback_info info) {
napi_value result_symbol;
NODE_API_CALL(env, node_api_symbol_for(env, NULL, 0, &result_symbol));
return result_symbol;
}

static napi_value CreateSymbolForIncorrectLength(napi_env env, napi_callback_info info) {
napi_value result_symbol;
NODE_API_CALL(env, node_api_symbol_for(env, NULL, 5, &result_symbol));
return result_symbol;
}

static napi_value
CreateExternalWithFinalize(napi_env env, napi_callback_info info) {
napi_value result;
Expand Down Expand Up @@ -190,6 +226,9 @@ napi_value Init(napi_env env, napi_value exports) {
DECLARE_NODE_API_PROPERTY("checkExternal", CheckExternal),
DECLARE_NODE_API_PROPERTY("createReference", CreateReference),
DECLARE_NODE_API_PROPERTY("createSymbol", CreateSymbol),
DECLARE_NODE_API_PROPERTY("createSymbolFor", CreateSymbolFor),
DECLARE_NODE_API_PROPERTY("createSymbolForEmptyString", CreateSymbolForEmptyString),
DECLARE_NODE_API_PROPERTY("createSymbolForIncorrectLength", CreateSymbolForIncorrectLength),
DECLARE_NODE_API_PROPERTY("deleteReference", DeleteReference),
DECLARE_NODE_API_PROPERTY("incrementRefcount", IncrementRefcount),
DECLARE_NODE_API_PROPERTY("decrementRefcount", DecrementRefcount),
Expand Down