src: improve JS Array creation from C++

src/node.cc

@@ -932,28 +932,15 @@ void SetupProcessObject(Environment* env,
 #endif
 
   // process.argv
-  Local<Array> arguments = Array::New(env->isolate(), args.size());
-  for (size_t i = 0; i < args.size(); ++i) {
-    arguments->Set(env->context(), i,
-        String::NewFromUtf8(env->isolate(), args[i].c_str(),
-                            NewStringType::kNormal).ToLocalChecked())
-        .FromJust();
-  }
   process->Set(env->context(),
                FIXED_ONE_BYTE_STRING(env->isolate(), "argv"),
-               arguments).FromJust();
+               ToV8Value(env->context(), args).ToLocalChecked()).FromJust();
 
   // process.execArgv
-  Local<Array> exec_arguments = Array::New(env->isolate(), exec_args.size());
-  for (size_t i = 0; i < exec_args.size(); ++i) {
-    exec_arguments->Set(env->context(), i,
-        String::NewFromUtf8(env->isolate(), exec_args[i].c_str(),
-                            NewStringType::kNormal).ToLocalChecked())
-        .FromJust();
-  }
   process->Set(env->context(),
                FIXED_ONE_BYTE_STRING(env->isolate(), "execArgv"),
-               exec_arguments).FromJust();
+               ToV8Value(env->context(), exec_args)
+                   .ToLocalChecked()).FromJust();
 
   // create process.env
   process
@@ -1004,17 +991,10 @@ void SetupProcessObject(Environment* env,
   const std::vector<std::string>& preload_modules =
       env->options()->preload_modules;
   if (!preload_modules.empty()) {
-    Local<Array> array = Array::New(env->isolate());
-    for (unsigned int i = 0; i < preload_modules.size(); ++i) {
-      Local<String> module = String::NewFromUtf8(env->isolate(),
-                                                 preload_modules[i].c_str(),
-                                                 NewStringType::kNormal)
-                                 .ToLocalChecked();
-      array->Set(env->context(), i, module).FromJust();
-    }
     READONLY_PROPERTY(process,
                       "_preload_modules",
-                      array);
+                      ToV8Value(env->context(), preload_modules)
+                          .ToLocalChecked());
   }
 
   // --no-deprecation

src/node_credentials.cc

@@ -15,9 +15,9 @@ using v8::Array;
 using v8::Context;
 using v8::Function;
 using v8::FunctionCallbackInfo;
-using v8::Integer;
 using v8::Isolate;
 using v8::Local;
+using v8::MaybeLocal;
 using v8::Object;
 using v8::String;
 using v8::Uint32;
@@ -253,36 +253,21 @@ static void GetGroups(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
   int ngroups = getgroups(0, nullptr);
-
   if (ngroups == -1) return env->ThrowErrnoException(errno, "getgroups");
 
-  gid_t* groups = new gid_t[ngroups];
-
-  ngroups = getgroups(ngroups, groups);
+  std::vector<gid_t> groups(ngroups);
 
-  if (ngroups == -1) {
-    delete[] groups;
+  ngroups = getgroups(ngroups, groups.data());
+  if (ngroups == -1)
     return env->ThrowErrnoException(errno, "getgroups");
-  }
 
-  Local<Array> groups_list = Array::New(env->isolate(), ngroups);
-  bool seen_egid = false;
+  groups.resize(ngroups);
   gid_t egid = getegid();
-
-  for (int i = 0; i < ngroups; i++) {
-    groups_list->Set(env->context(), i, Integer::New(env->isolate(), groups[i]))
-        .FromJust();
-    if (groups[i] == egid) seen_egid = true;
-  }
-
-  delete[] groups;
-
-  if (seen_egid == false)
-    groups_list
-        ->Set(env->context(), ngroups, Integer::New(env->isolate(), egid))
-        .FromJust();
-
-  args.GetReturnValue().Set(groups_list);
+  if (std::find(groups.begin(), groups.end(), egid) == groups.end())
+    groups.push_back(egid);
+  MaybeLocal<Value> array = ToV8Value(env->context(), groups);
+  if (!array.IsEmpty())
+    args.GetReturnValue().Set(array.ToLocalChecked());
 }
 
 static void SetGroups(const FunctionCallbackInfo<Value>& args) {

src/node_crypto.cc

@@ -250,22 +250,12 @@ struct CryptoErrorVector : public std::vector<std::string> {
     CHECK(!exception_v.IsEmpty());
 
     if (!empty()) {
-      Local<Array> array = Array::New(env->isolate(), size());
-      CHECK(!array.IsEmpty());
-
-      for (const std::string& string : *this) {
-        const size_t index = &string - &front();
-        Local<String> value =
-            String::NewFromUtf8(env->isolate(), string.data(),
-                                NewStringType::kNormal, string.size())
-            .ToLocalChecked();
-        array->Set(env->context(), index, value).FromJust();
-      }
-
       CHECK(exception_v->IsObject());
       Local<Object> exception = exception_v.As<Object>();
       exception->Set(env->context(),
-                     env->openssl_error_stack(), array).FromJust();
+                     env->openssl_error_stack(),
+                     ToV8Value(env->context(), *this).ToLocalChecked())
+          .FromJust();
     }
 
     return exception_v;

src/node_url.cc

@@ -1190,15 +1190,6 @@ inline std::vector<std::string> FromJSStringArray(Environment* env,
   return vec;
 }
 
-inline Local<Array> ToJSStringArray(Environment* env,
-                                    const std::vector<std::string>& vec) {
-  Isolate* isolate = env->isolate();
-  Local<Array> array = Array::New(isolate, vec.size());
-  for (size_t n = 0; n < vec.size(); n++)
-    array->Set(env->context(), n, Utf8String(isolate, vec[n])).FromJust();
-  return array;
-}
-
 inline url_data HarvestBase(Environment* env, Local<Object> base_obj) {
   url_data base;
   Local<Context> context = env->context();
@@ -2119,7 +2110,7 @@ static inline void SetArgs(Environment* env,
   if (url.port > -1)
     argv[ARG_PORT] = Integer::New(isolate, url.port);
   if (url.flags & URL_FLAGS_HAS_PATH)
-    argv[ARG_PATH] = ToJSStringArray(env, url.path);
+    argv[ARG_PATH] = ToV8Value(env->context(), url.path).ToLocalChecked();
 }
 
 static void Parse(Environment* env,

src/util-inl.h

@@ -377,8 +377,9 @@ inline char* UncheckedCalloc(size_t n) { return UncheckedCalloc<char>(n); }
 void ThrowErrStringTooLong(v8::Isolate* isolate);
 
 v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
-                                    const std::string& str) {
-  v8::Isolate* isolate = context->GetIsolate();
+                                    const std::string& str,
+                                    v8::Isolate* isolate) {
+  if (isolate == nullptr) isolate = context->GetIsolate();
   if (UNLIKELY(str.size() >= static_cast<size_t>(v8::String::kMaxLength))) {
     // V8 only has a TODO comment about adding an exception when the maximum
     // string size is exceeded.
@@ -393,33 +394,33 @@ v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
 
 template <typename T>
 v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
-                                    const std::vector<T>& vec) {
-  v8::Isolate* isolate = context->GetIsolate();
+                                    const std::vector<T>& vec,
+                                    v8::Isolate* isolate) {
+  if (isolate == nullptr) isolate = context->GetIsolate();
   v8::EscapableHandleScope handle_scope(isolate);
 
-  v8::Local<v8::Array> arr = v8::Array::New(isolate, vec.size());
+  MaybeStackBuffer<v8::Local<v8::Value>, 128> arr(vec.size());
+  arr.SetLength(vec.size());
   for (size_t i = 0; i < vec.size(); ++i) {
-    v8::Local<v8::Value> val;
-    if (!ToV8Value(context, vec[i]).ToLocal(&val) ||
-        arr->Set(context, i, val).IsNothing()) {
+    if (!ToV8Value(context, vec[i], isolate).ToLocal(&arr[i]))
       return v8::MaybeLocal<v8::Value>();
-    }
   }
 
-  return handle_scope.Escape(arr);
+  return handle_scope.Escape(v8::Array::New(isolate, arr.out(), arr.length()));
 }
 
 template <typename T, typename U>
 v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
-                                    const std::unordered_map<T, U>& map) {
-  v8::Isolate* isolate = context->GetIsolate();
+                                    const std::unordered_map<T, U>& map,
+                                    v8::Isolate* isolate) {
+  if (isolate == nullptr) isolate = context->GetIsolate();
   v8::EscapableHandleScope handle_scope(isolate);
 
   v8::Local<v8::Map> ret = v8::Map::New(isolate);
   for (const auto& item : map) {
     v8::Local<v8::Value> first, second;
-    if (!ToV8Value(context, item.first).ToLocal(&first) ||
-        !ToV8Value(context, item.second).ToLocal(&second) ||
+    if (!ToV8Value(context, item.first, isolate).ToLocal(&first) ||
+        !ToV8Value(context, item.second, isolate).ToLocal(&second) ||
         ret->Set(context, first, second).IsEmpty()) {
       return v8::MaybeLocal<v8::Value>();
     }
@@ -428,6 +429,32 @@ v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
   return handle_scope.Escape(ret);
 }
 
+template <typename T, typename >
+v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
+                                    const T& number,
+                                    v8::Isolate* isolate) {
+  if (isolate == nullptr) isolate = context->GetIsolate();
+
+  using Limits = std::numeric_limits<T>;
+  // Choose Uint32, Int32, or Double depending on range checks.
+  // These checks should all collapse at compile time.
+  if (static_cast<uint32_t>(Limits::max()) <=
+          std::numeric_limits<uint32_t>::max() &&
+      static_cast<uint32_t>(Limits::min()) >=
+          std::numeric_limits<uint32_t>::min() && Limits::is_exact) {
+    return v8::Integer::NewFromUnsigned(isolate, static_cast<uint32_t>(number));
+  }
+
+  if (static_cast<int32_t>(Limits::max()) <=
+          std::numeric_limits<int32_t>::max() &&
+      static_cast<int32_t>(Limits::min()) >=
+          std::numeric_limits<int32_t>::min() && Limits::is_exact) {
+    return v8::Integer::New(isolate, static_cast<int32_t>(number));
+  }
+
+  return v8::Number::New(isolate, static_cast<double>(number));
+}
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

src/util.h

@@ -35,6 +35,7 @@
 #include <string.h>
 
 #include <functional>  // std::function
+#include <limits>
 #include <set>
 #include <string>
 #include <array>
@@ -519,13 +520,21 @@ using DeleteFnPtr = typename FunctionDeleter<T, function>::Pointer;
 std::set<std::string> ParseCommaSeparatedSet(const std::string& in);
 
 inline v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
-                                           const std::string& str);
+                                           const std::string& str,
+                                           v8::Isolate* isolate = nullptr);
+template <typename T, typename test_for_number =
+    typename std::enable_if<std::numeric_limits<T>::is_specialized, bool>::type>
+inline v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
+                                           const T& number,
+                                           v8::Isolate* isolate = nullptr);
 template <typename T>
 inline v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
-                                           const std::vector<T>& vec);
+                                           const std::vector<T>& vec,
+                                           v8::Isolate* isolate = nullptr);
 template <typename T, typename U>
 inline v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
-                                           const std::unordered_map<T, U>& map);
+                                           const std::unordered_map<T, U>& map,
+                                           v8::Isolate* isolate = nullptr);
 
 // These macros expects a `Isolate* isolate` and a `Local<Context> context`
 // to be in the scope.