node-api: use WriteV2 in napi_get_value_string_utf16 #58165
Conversation
Since `String::Write()` is deprecated, use `String::Write2()` instead. That requires us to compute the correct number of characters ahead of time but removes the need for dealing with the return value.
tniessen
added
c++
|
Review requested:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #58165 +/- ##
==========================================
- Coverage 90.17% 90.16% -0.02%
==========================================
Files 630 630
Lines 186503 186504 +1
Branches 36614 36615 +1
==========================================
- Hits 168187 168163 -24
- Misses 11124 11128 +4
- Partials 7192 7213 +21
🚀 New features to boost your workflow:
|
|
Could you also replace the other deprecated |
| 0, | ||
| bufsize - 1, | ||
| v8::String::NO_NULL_TERMINATION); | ||
| size_t n_chars = std::min( |
There was a problem hiding this comment.
nit: the n_chars is a bit cryptic and unusual in this context. Could you use something like length, str_length, str_len, or str_size. The WriteV2 function parameter name is length.
| bufsize - 1, static_cast<size_t>(val.As<v8::String>()->Length())); | ||
| val.As<v8::String>()->WriteV2(env->isolate, | ||
| 0, | ||
| n_chars, |
There was a problem hiding this comment.
The WriteV2 function parameter length is uint32_t. It is different from the size_t on 64-bit CPUs. Please use the uint32_t type to avoid compiler warnings.
| bufsize - 1, | ||
| v8::String::NO_NULL_TERMINATION); | ||
| size_t n_chars = std::min( | ||
| bufsize - 1, static_cast<size_t>(val.As<v8::String>()->Length())); |
There was a problem hiding this comment.
It would be nice to avoid calling val.As<v8::String>() twice.
In case if it does additional checks then it may cause unnecessary overhead.
Since
String::Write()is deprecated, useString::Write2()instead. That requires us to compute the correct number of characters ahead of time but removes the need for dealing with the return value.