Skip to content

Commit 468275a

Browse files
jasonginmhdawson
authored andcommitted
n-api: remove napi_get_value_string_length()
This API doesn't serve much purpose, and is only likely to cause confusion and bugs. The intention was that this would return the number of characters in a string independent of encoding, but that's not generally useful. In almost all cases, one of the encoding-specific napi_get_value_string_* APIs is more correct. (Pass a null buffer if only the encoded length is desired.) Anyway the current implementation of napi_get_value_string_length() is technically wrong: it returns the number of 2-byte code units of the UTF-16 encoding, but there are actually some characters that are encoded as two UTF-16 code units. Note the JavaScript String.prototype.length property returns the number of UTF-16 code units, which may be different from the number of characters. So, getting the true character count is not common with JavaScript, and is probably best left to specialized internationalization libraries. PR-URL: #12496 Fixes: nodejs/abi-stable-node#226 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent ce986de commit 468275a

File tree

4 files changed

+10
-30
lines changed

4 files changed

+10
-30
lines changed

src/node_api.cc

-15
Original file line numberDiff line numberDiff line change
@@ -1719,21 +1719,6 @@ napi_status napi_get_value_bool(napi_env env, napi_value value, bool* result) {
17191719
return napi_ok;
17201720
}
17211721

1722-
// Gets the number of CHARACTERS in the string.
1723-
napi_status napi_get_value_string_length(napi_env env,
1724-
napi_value value,
1725-
size_t* result) {
1726-
NAPI_PREAMBLE(env);
1727-
CHECK_ARG(env, result);
1728-
1729-
v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
1730-
RETURN_STATUS_IF_FALSE(env, val->IsString(), napi_string_expected);
1731-
1732-
*result = val.As<v8::String>()->Length();
1733-
1734-
return GET_RETURN_STATUS(env);
1735-
}
1736-
17371722
// Copies a JavaScript string into a LATIN-1 string buffer. The result is the
17381723
// number of bytes (excluding the null terminator) copied into buf.
17391724
// A sufficient buffer size should be greater than the length of string,

src/node_api.h

-5
Original file line numberDiff line numberDiff line change
@@ -171,11 +171,6 @@ NAPI_EXTERN napi_status napi_get_value_bool(napi_env env,
171171
napi_value value,
172172
bool* result);
173173

174-
// Gets the number of CHARACTERS in the string.
175-
NAPI_EXTERN napi_status napi_get_value_string_length(napi_env env,
176-
napi_value value,
177-
size_t* result);
178-
179174
// Copies LATIN-1 encoded bytes from a string into a buffer.
180175
NAPI_EXTERN napi_status napi_get_value_string_latin1(napi_env env,
181176
napi_value value,

test/addons-napi/test_string/test.js

+7-7
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const empty = '';
99
assert.strictEqual(test_string.TestLatin1(empty), empty);
1010
assert.strictEqual(test_string.TestUtf8(empty), empty);
1111
assert.strictEqual(test_string.TestUtf16(empty), empty);
12-
assert.strictEqual(test_string.Length(empty), 0);
12+
assert.strictEqual(test_string.Utf16Length(empty), 0);
1313
assert.strictEqual(test_string.Utf8Length(empty), 0);
1414

1515
const str1 = 'hello world';
@@ -19,7 +19,7 @@ assert.strictEqual(test_string.TestUtf16(str1), str1);
1919
assert.strictEqual(test_string.TestLatin1Insufficient(str1), str1.slice(0, 3));
2020
assert.strictEqual(test_string.TestUtf8Insufficient(str1), str1.slice(0, 3));
2121
assert.strictEqual(test_string.TestUtf16Insufficient(str1), str1.slice(0, 3));
22-
assert.strictEqual(test_string.Length(str1), 11);
22+
assert.strictEqual(test_string.Utf16Length(str1), 11);
2323
assert.strictEqual(test_string.Utf8Length(str1), 11);
2424

2525
const str2 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
@@ -29,7 +29,7 @@ assert.strictEqual(test_string.TestUtf16(str2), str2);
2929
assert.strictEqual(test_string.TestLatin1Insufficient(str2), str2.slice(0, 3));
3030
assert.strictEqual(test_string.TestUtf8Insufficient(str2), str2.slice(0, 3));
3131
assert.strictEqual(test_string.TestUtf16Insufficient(str2), str2.slice(0, 3));
32-
assert.strictEqual(test_string.Length(str2), 62);
32+
assert.strictEqual(test_string.Utf16Length(str2), 62);
3333
assert.strictEqual(test_string.Utf8Length(str2), 62);
3434

3535
const str3 = '?!@#$%^&*()_+-=[]{}/.,<>\'"\\';
@@ -39,7 +39,7 @@ assert.strictEqual(test_string.TestUtf16(str3), str3);
3939
assert.strictEqual(test_string.TestLatin1Insufficient(str3), str3.slice(0, 3));
4040
assert.strictEqual(test_string.TestUtf8Insufficient(str3), str3.slice(0, 3));
4141
assert.strictEqual(test_string.TestUtf16Insufficient(str3), str3.slice(0, 3));
42-
assert.strictEqual(test_string.Length(str3), 27);
42+
assert.strictEqual(test_string.Utf16Length(str3), 27);
4343
assert.strictEqual(test_string.Utf8Length(str3), 27);
4444

4545
const str4 = '¡¢£¤¥¦§¨©ª«¬­®¯°±²³´µ¶·¸¹º»¼½¾¿';
@@ -49,7 +49,7 @@ assert.strictEqual(test_string.TestUtf16(str4), str4);
4949
assert.strictEqual(test_string.TestLatin1Insufficient(str4), str4.slice(0, 3));
5050
assert.strictEqual(test_string.TestUtf8Insufficient(str4), str4.slice(0, 1));
5151
assert.strictEqual(test_string.TestUtf16Insufficient(str4), str4.slice(0, 3));
52-
assert.strictEqual(test_string.Length(str4), 31);
52+
assert.strictEqual(test_string.Utf16Length(str4), 31);
5353
assert.strictEqual(test_string.Utf8Length(str4), 62);
5454

5555
const str5 = 'ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖ×ØÙÚÛÜÝÞßàáâãäåæçèéêëìíîïðñòóôõö÷øùúûüýþ';
@@ -59,13 +59,13 @@ assert.strictEqual(test_string.TestUtf16(str5), str5);
5959
assert.strictEqual(test_string.TestLatin1Insufficient(str5), str5.slice(0, 3));
6060
assert.strictEqual(test_string.TestUtf8Insufficient(str5), str5.slice(0, 1));
6161
assert.strictEqual(test_string.TestUtf16Insufficient(str5), str5.slice(0, 3));
62-
assert.strictEqual(test_string.Length(str5), 63);
62+
assert.strictEqual(test_string.Utf16Length(str5), 63);
6363
assert.strictEqual(test_string.Utf8Length(str5), 126);
6464

6565
const str6 = '\u{2003}\u{2101}\u{2001}\u{202}\u{2011}';
6666
assert.strictEqual(test_string.TestUtf8(str6), str6);
6767
assert.strictEqual(test_string.TestUtf16(str6), str6);
6868
assert.strictEqual(test_string.TestUtf8Insufficient(str6), str6.slice(0, 1));
6969
assert.strictEqual(test_string.TestUtf16Insufficient(str6), str6.slice(0, 3));
70-
assert.strictEqual(test_string.Length(str6), 5);
70+
assert.strictEqual(test_string.Utf16Length(str6), 5);
7171
assert.strictEqual(test_string.Utf8Length(str6), 14);

test/addons-napi/test_string/test_string.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ napi_value TestUtf16Insufficient(napi_env env, napi_callback_info info) {
157157
return output;
158158
}
159159

160-
napi_value Length(napi_env env, napi_callback_info info) {
160+
napi_value Utf16Length(napi_env env, napi_callback_info info) {
161161
size_t argc = 1;
162162
napi_value args[1];
163163
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
@@ -171,7 +171,7 @@ napi_value Length(napi_env env, napi_callback_info info) {
171171
"Wrong type of argment. Expects a string.");
172172

173173
size_t length;
174-
NAPI_CALL(env, napi_get_value_string_length(env, args[0], &length));
174+
NAPI_CALL(env, napi_get_value_string_utf16(env, args[0], NULL, 0, &length));
175175

176176
napi_value output;
177177
NAPI_CALL(env, napi_create_number(env, (double)length, &output));
@@ -209,7 +209,7 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
209209
DECLARE_NAPI_PROPERTY("TestUtf8Insufficient", TestUtf8Insufficient),
210210
DECLARE_NAPI_PROPERTY("TestUtf16", TestUtf16),
211211
DECLARE_NAPI_PROPERTY("TestUtf16Insufficient", TestUtf16Insufficient),
212-
DECLARE_NAPI_PROPERTY("Length", Length),
212+
DECLARE_NAPI_PROPERTY("Utf16Length", Utf16Length),
213213
DECLARE_NAPI_PROPERTY("Utf8Length", Utf8Length),
214214
};
215215

0 commit comments

Comments
 (0)