Repository: nodejs/node
Pull request: Update std::vector<v8::Local<T>> to use v8::LocalVector<T> (Part 2)

A follow up of https://github.com/nodejs/node/pull/57578 to replace all `std::vector<v8::Local<T>>` to use `v8::LocalVector<T>`

Also updates `src/README.md` to document the same.

Changed files:
filename: src/README.md
patch: @@ -151,6 +151,25 @@ is done executing. `Local` handles can only be allocated on the C++ stack.
 Most of the V8 API uses `Local` handles to work with JavaScript values or return
 them from functions.
 
+Additionally, according to [V8 public API documentation][`v8::Local<T>`], local handles
+(`v8::Local<T>`) should **never** be allocated on the heap.
+
+This disallows heap-allocated data structures containing instances of `v8::Local`
+
+For example:
+
+```cpp
+// Don't do this
+std::vector<v8::Local<v8::Value>> v1;
+```
+
+Instead, it is recommended to use `v8::LocalVector<T>` provided by V8
+for such scenarios:
+
+```cpp
+v8::LocalVector<v8::Value> v1(isolate);
+```
+
 Whenever a `Local` handle is created, a `v8::HandleScope` or
 `v8::EscapableHandleScope` object must exist on the stack. The `Local` is then
 added to that scope and deleted along with it.
@@ -1409,6 +1428,7 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
 [`v8.h` in Code Search]: https://cs.chromium.org/chromium/src/v8/include/v8.h
 [`v8.h` in Node.js]: https://github.com/nodejs/node/blob/HEAD/deps/v8/include/v8.h
 [`v8.h` in V8]: https://github.com/v8/v8/blob/HEAD/include/v8.h
+[`v8::Local<T>`]: https://v8.github.io/api/head/classv8_1_1Local.html
 [`vm` module]: https://nodejs.org/api/vm.html
 [binding function]: #binding-functions
 [cleanup hooks]: #cleanup-hooks

filename: src/node_contextify.cc
patch: @@ -1166,7 +1166,7 @@ Maybe<void> StoreCodeCacheResult(
 MaybeLocal<Function> CompileFunction(Local<Context> context,
                                      Local<String> filename,
                                      Local<String> content,
-                                     std::vector<Local<String>>* parameters) {
+                                     LocalVector<String>* parameters) {
   ScriptOrigin script_origin(filename, 0, 0, true);
   ScriptCompiler::Source script_source(content, script_origin);
 
@@ -1483,7 +1483,7 @@ void ContextifyContext::CompileFunction(
   Context::Scope scope(parsing_context);
 
   // Read context extensions from buffer
-  std::vector<Local<Object>> context_extensions;
+  LocalVector<Object> context_extensions(isolate);
   if (!context_extensions_buf.IsEmpty()) {
     for (uint32_t n = 0; n < context_extensions_buf->Length(); n++) {
       Local<Value> val;
@@ -1494,7 +1494,7 @@ void ContextifyContext::CompileFunction(
   }
 
   // Read params from params buffer
-  std::vector<Local<String>> params;
+  LocalVector<String> params(isolate);
   if (!params_buf.IsEmpty()) {
     for (uint32_t n = 0; n < params_buf->Length(); n++) {
       Local<Value> val;
@@ -1526,22 +1526,24 @@ void ContextifyContext::CompileFunction(
   args.GetReturnValue().Set(result);
 }
 
-static std::vector<Local<String>> GetCJSParameters(IsolateData* data) {
-  return {
-      data->exports_string(),
-      data->require_string(),
-      data->module_string(),
-      data->__filename_string(),
-      data->__dirname_string(),
-  };
+static LocalVector<String> GetCJSParameters(IsolateData* data) {
+  LocalVector<String> result(data->isolate(),
+                             {
+                                 data->exports_string(),
+                                 data->require_string(),
+                                 data->module_string(),
+                                 data->__filename_string(),
+                                 data->__dirname_string(),
+                             });
+  return result;
 }
 
 Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
     Environment* env,
     Local<Context> parsing_context,
     ScriptCompiler::Source* source,
-    std::vector<Local<String>> params,
-    std::vector<Local<Object>> context_extensions,
+    LocalVector<String> params,
+    LocalVector<Object> context_extensions,
     ScriptCompiler::CompileOptions options,
     bool produce_cached_data,
     Local<Symbol> id_symbol,
@@ -1677,7 +1679,7 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(
     options = ScriptCompiler::kConsumeCodeCache;
   }
 
-  std::vector<Local<String>> params;
+  LocalVector<String> params(isolate);
   if (is_cjs_scope) {
     params = GetCJSParameters(env->isolate_data());
   }

filename: src/node_contextify.h
patch: @@ -149,8 +149,8 @@ class ContextifyContext final : CPPGC_MIXIN(ContextifyContext) {
       Environment* env,
       v8::Local<v8::Context> parsing_context,
       v8::ScriptCompiler::Source* source,
-      std::vector<v8::Local<v8::String>> params,
-      std::vector<v8::Local<v8::Object>> context_extensions,
+      v8::LocalVector<v8::String> params,
+      v8::LocalVector<v8::Object> context_extensions,
       v8::ScriptCompiler::CompileOptions options,
       bool produce_cached_data,
       v8::Local<v8::Symbol> id_symbol,
@@ -244,7 +244,7 @@ v8::MaybeLocal<v8::Function> CompileFunction(
     v8::Local<v8::Context> context,
     v8::Local<v8::String> filename,
     v8::Local<v8::String> content,
-    std::vector<v8::Local<v8::String>>* parameters);
+    v8::LocalVector<v8::String>* parameters);
 
 }  // namespace contextify
 }  // namespace node

filename: src/node_sea.cc
patch: @@ -36,6 +36,7 @@ using v8::FunctionCallbackInfo;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
+using v8::LocalVector;
 using v8::MaybeLocal;
 using v8::NewStringType;
 using v8::Object;
@@ -450,13 +451,15 @@ std::optional<std::string> GenerateCodeCache(std::string_view main_path,
     return std::nullopt;
   }
 
-  std::vector<Local<String>> parameters = {
-      FIXED_ONE_BYTE_STRING(isolate, "exports"),
-      FIXED_ONE_BYTE_STRING(isolate, "require"),
-      FIXED_ONE_BYTE_STRING(isolate, "module"),
-      FIXED_ONE_BYTE_STRING(isolate, "__filename"),
-      FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
-  };
+  LocalVector<String> parameters(
+      isolate,
+      {
+          FIXED_ONE_BYTE_STRING(isolate, "exports"),
+          FIXED_ONE_BYTE_STRING(isolate, "require"),
+          FIXED_ONE_BYTE_STRING(isolate, "module"),
+          FIXED_ONE_BYTE_STRING(isolate, "__filename"),
+          FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
+      });
 
   // TODO(RaisinTen): Using the V8 code cache prevents us from using `import()`
   // in the SEA code. Support it.

filename: src/node_snapshotable.cc
patch: @@ -41,6 +41,7 @@ using v8::FunctionCallbackInfo;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
+using v8::LocalVector;
 using v8::Object;
 using v8::ObjectTemplate;
 using v8::SnapshotCreator;
@@ -1480,11 +1481,13 @@ void CompileSerializeMain(const FunctionCallbackInfo<Value>& args) {
   Local<Context> context = isolate->GetCurrentContext();
   // TODO(joyeecheung): do we need all of these? Maybe we would want a less
   // internal version of them.
-  std::vector<Local<String>> parameters = {
-      FIXED_ONE_BYTE_STRING(isolate, "require"),
-      FIXED_ONE_BYTE_STRING(isolate, "__filename"),
-      FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
-  };
+  LocalVector<String> parameters(
+      isolate,
+      {
+          FIXED_ONE_BYTE_STRING(isolate, "require"),
+          FIXED_ONE_BYTE_STRING(isolate, "__filename"),
+          FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
+      });
   Local<Function> fn;
   if (contextify::CompileFunction(context, filename, source, &parameters)
           .ToLocal(&fn)) {