src: add a variant of ToV8Value()

[Aditi-1400]

Adds a variant of ToV8Value for array of primitives that do not need to throw during conversion - there is essentially no exceptions that can be thrown then you create an array of integers and you are only dealing with the one-size-fit-all abstraction of ToV8Value.

[jasnell]

While I'm fine with adding a new ToV8Value variant in general, I'd prefer to do so only if there are places it is used. Are there existing call sites that would use this?

[Aditi-1400]

Yes, it can be used in ConvertHeapStatsToJSObject, we can replace:

node/src/node_v8.cc

Lines 388 to 404 in a46af03

	Local<Value> bucket_size_value;
	if (!ToV8Value(context, space_stats.free_list_stats.bucket_size)
	.ToLocal(&bucket_size_value)) {
	return MaybeLocal<Object>();
	}
	Local<Value> free_count_value;
	if (!ToV8Value(context, space_stats.free_list_stats.free_count)
	.ToLocal(&free_count_value)) {
	return MaybeLocal<Object>();
	}
	Local<Value> free_size_value;
	if (!ToV8Value(context, space_stats.free_list_stats.free_size)
	.ToLocal(&free_size_value)) {
	return MaybeLocal<Object>();
	}
	Local<Value> free_list_statistics_values[] = {
	bucket_size_value, free_count_value, free_size_value};

with:

   Local<Value> free_list_statistics_values[] = {
      ToV8ValuePrimitiveArray(context, space_stats.free_list_stats.bucket_size, isolate),
      ToV8ValuePrimitiveArray(context, space_stats.free_list_stats.free_count, isolate),
      ToV8ValuePrimitiveArray(context, space_stats.free_list_stats.free_size, isolate)
    };

[anonrig]

This won't be the same exact replacement, since if one of the ToV8Value calls fail, it returns and stop execution. But in your example and code, it throws.

[joyeecheung]

Creating primitives and arrays of primitives do not actually throw - both v8::Array::New and v8::Integer::New have non-throwing variants that just copy things, for example. So it's just a variant of ToV8Value for converting values that would not have thrown in the first place, and would be more efficient than using the old one because we were dealing with non-existent empty maybes just because the old API was too one-size-fits-all.

[jasnell]

@Aditi-1400 ... my point on the usage is that we shouldn't land this PR without those usages in place. If this can be used to improve various callsites, then those should be updated here also.

[jasnell]

Using a v8::LocalVector and creating the v8::Array and the end without using the Set(...) method would be better here.

[jasnell]

We need to avoid using ToLocalChecked() and instead properly propagate the error up to avoid needlessly crashing the process.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.23%. Comparing base (922ce9d) to head (fc7eafe).
Report is 23 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57576      +/-   ##
==========================================
- Coverage   90.23%   90.23%   -0.01%     
==========================================
  Files         629      630       +1     
  Lines      184939   185013      +74     
  Branches    36232    36244      +12     
==========================================
+ Hits       166885   166944      +59     
+ Misses      11011    11002       -9     
- Partials     7043     7067      +24     

Files with missing lines	Coverage Δ
src/util-inl.h	83.27% <ø> (ø)

... and 44 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[anonrig]

This is not a better replacement than what we have right now, and lacks real world usage. We shouldn't introduce new functions that we don't use at all.