src,lib: group properties used as constants from util binding

Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com> PR-URL: #45539 Backport-PR-URL: #45674 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
nodejs · Jan 3, 2023 · 90e8418 · 90e8418
1 parent 3f18a83
commit 90e8418
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 36 deletions.
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -67,11 +67,11 @@ const {
   kStringMaxLength
 } = internalBinding('buffer');
 const {
-  getOwnNonIndexProperties,
-  propertyFilter: {
+  constants: {
     ALL_PROPERTIES,
-    ONLY_ENUMERABLE
+    ONLY_ENUMERABLE,
   },
+  getOwnNonIndexProperties,
 } = internalBinding('util');
 const {
   customInspectSymbol,

diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js
@@ -46,11 +46,11 @@ const {
   isFloat64Array,
 } = types;
 const {
-  getOwnNonIndexProperties,
-  propertyFilter: {
+  constants: {
     ONLY_ENUMERABLE,
-    SKIP_SYMBOLS
-  }
+    SKIP_SYMBOLS,
+  },
+  getOwnNonIndexProperties,
 } = internalBinding('util');
 
 const kStrict = true;

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
@@ -97,18 +97,18 @@ const {
 } = primordials;
 
 const {
+  constants: {
+    ALL_PROPERTIES,
+    ONLY_ENUMERABLE,
+    kPending,
+    kRejected,
+  },
   getOwnNonIndexProperties,
   getPromiseDetails,
   getProxyDetails,
-  kPending,
-  kRejected,
   previewEntries,
   getConstructorName: internalGetConstructorName,
   getExternalValue,
-  propertyFilter: {
-    ALL_PROPERTIES,
-    ONLY_ENUMERABLE
-  }
 } = internalBinding('util');
 
 const {

diff --git a/lib/internal/webstreams/util.js b/lib/internal/webstreams/util.js
@@ -40,8 +40,10 @@ const {
 } = require('util');
 
 const {
+  constants: {
+    kPending,
+  },
   getPromiseDetails,
-  kPending,
 } = internalBinding('util');
 
 const assert = require('internal/assert');

diff --git a/lib/repl.js b/lib/repl.js
@@ -166,11 +166,11 @@ const {
   setupReverseSearch,
 } = require('internal/repl/utils');
 const {
-  getOwnNonIndexProperties,
-  propertyFilter: {
+  constants: {
     ALL_PROPERTIES,
-    SKIP_SYMBOLS
-  }
+    SKIP_SYMBOLS,
+  },
+  getOwnNonIndexProperties,
 } = internalBinding('util');
 const {
   startSigintWatchdog,

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -387,16 +387,38 @@ void Initialize(Local<Object> target,
         .Check();
   }
 
-#define V(name)                                                               \
-  target->Set(context,                                                        \
-              FIXED_ONE_BYTE_STRING(env->isolate(), #name),                   \
-              Integer::New(env->isolate(), Promise::PromiseState::name))      \
-    .FromJust()
-  V(kPending);
-  V(kFulfilled);
-  V(kRejected);
+  {
+    Local<Object> constants = Object::New(isolate);
+#define V(name)                                                                \
+  constants                                                                    \
+      ->Set(context,                                                           \
+            FIXED_ONE_BYTE_STRING(isolate, #name),                             \
+            Integer::New(isolate, Promise::PromiseState::name))                \
+      .Check();
+
+    V(kPending);
+    V(kFulfilled);
+    V(kRejected);
+#undef V
+
+#define V(name)                                                                \
+  constants                                                                    \
+      ->Set(context,                                                           \
+            FIXED_ONE_BYTE_STRING(isolate, #name),                             \
+            Integer::New(isolate, PropertyFilter::name))                       \
+      .Check();
+
+    V(ALL_PROPERTIES);
+    V(ONLY_WRITABLE);
+    V(ONLY_ENUMERABLE);
+    V(ONLY_CONFIGURABLE);
+    V(SKIP_STRINGS);
+    V(SKIP_SYMBOLS);
 #undef V
 
+    target->Set(context, env->constants_string(), constants).Check();
+  }
+
   SetMethodNoSideEffect(
       context, target, "getPromiseDetails", GetPromiseDetails);
   SetMethodNoSideEffect(context, target, "getProxyDetails", GetProxyDetails);
@@ -410,16 +432,6 @@ void Initialize(Local<Object> target,
 
   SetMethod(
       context, target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer);
-  Local<Object> constants = Object::New(env->isolate());
-  NODE_DEFINE_CONSTANT(constants, ALL_PROPERTIES);
-  NODE_DEFINE_CONSTANT(constants, ONLY_WRITABLE);
-  NODE_DEFINE_CONSTANT(constants, ONLY_ENUMERABLE);
-  NODE_DEFINE_CONSTANT(constants, ONLY_CONFIGURABLE);
-  NODE_DEFINE_CONSTANT(constants, SKIP_STRINGS);
-  NODE_DEFINE_CONSTANT(constants, SKIP_SYMBOLS);
-  target->Set(context,
-              FIXED_ONE_BYTE_STRING(env->isolate(), "propertyFilter"),
-              constants).Check();
 
   Local<String> should_abort_on_uncaught_toggle =
       FIXED_ONE_BYTE_STRING(env->isolate(), "shouldAbortOnUncaughtToggle");