fixup: address comments

nodejs · Aug 20, 2018 · 7be3845 · 7be3845
1 parent 3f26e1f
commit 7be3845
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 19 deletions.
diff --git a/lib/internal/util.js b/lib/internal/util.js
@@ -366,15 +366,6 @@ function isInsideNodeModules() {
   return false;
 }
 
-const kPropertyFilter = Object.freeze({
-  ALL_PROPERTIES: 0,
-  ONLY_WRITABLE: 1,
-  ONLY_ENUMERABLE: 2,
-  ONLY_CONFIGURABLE: 4,
-  SKIP_STRINGS: 8,
-  SKIP_SYMBOLS: 16
-});
-
 module.exports = {
   assertCrypto,
   cachedResult,
@@ -394,7 +385,6 @@ module.exports = {
   promisify,
   spliceOne,
   removeColors,
-  kPropertyFilter,
 
   // Symbol used to customize promisify conversion
   customPromisifyArgs: kCustomPromisifyArgsSymbol,

diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js
@@ -10,8 +10,12 @@ const {
   isRegExp,
   isSet
 } = internalBinding('types');
-const { getOwnNonIndexProperties } = process.binding('util');
-const { kPropertyFilter: { ONLY_ENUMERABLE } } = require('internal/util');
+const {
+  getOwnNonIndexProperties,
+  propertyFilter: {
+    ONLY_ENUMERABLE
+  }
+} = process.binding('util');
 
 const ReflectApply = Reflect.apply;
 

diff --git a/lib/repl.js b/lib/repl.js
@@ -73,7 +73,13 @@ const {
 const { sendInspectorCommand } = require('internal/util/inspector');
 const { experimentalREPLAwait } = process.binding('config');
 const { isRecoverableError } = require('internal/repl/recoverable');
-const { getOwnNonIndexProperties } = process.binding('util');
+const {
+  getOwnNonIndexProperties,
+  propertyFilter: {
+    ALL_PROPERTIES,
+    SKIP_SYMBOLS
+  }
+} = process.binding('util');
 
 // Lazy-loaded.
 let processTopLevelAwait;
@@ -929,8 +935,7 @@ function isIdentifier(str) {
 
 function filteredOwnPropertyNames(obj) {
   if (!obj) return [];
-  const { kPropertyFilter } = internalUtil;
-  const filter = kPropertyFilter.ALL_PROPERTIES | kPropertyFilter.SKIP_SYMBOLS;
+  const filter = ALL_PROPERTIES | SKIP_SYMBOLS;
   return getOwnNonIndexProperties(obj, filter).filter(isIdentifier);
 }
 

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -14,23 +14,29 @@ using v8::Private;
 using v8::Promise;
 using v8::Proxy;
 using v8::String;
+using v8::Uint32;
 using v8::Value;
+using v8::ALL_PROPERTIES;
+using v8::ONLY_WRITABLE;
+using v8::ONLY_ENUMERABLE;
+using v8::ONLY_CONFIGURABLE;
+using v8::SKIP_STRINGS;
+using v8::SKIP_SYMBOLS;
 
 static void GetOwnNonIndexProperties(
     const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Local<Context> context = env->context();
 
-  if (!args[0]->IsObject() || !args[1]->IsUint32())
-    return;
+  CHECK(args[0]->IsObject());
+  CHECK(args[1]->IsUint32());
 
   Local<Object> object = args[0].As<Object>();
 
   Local<Array> properties;
 
   v8::PropertyFilter filter =
-    static_cast<v8::PropertyFilter>(
-      args[1]->Uint32Value(env->context()).FromJust());
+    static_cast<v8::PropertyFilter>(args[1].As<Uint32>()->Value());
 
   if (!object->GetPropertyNames(
         context, v8::KeyCollectionMode::kOwnOnly,
@@ -212,6 +218,17 @@ void Initialize(Local<Object> target,
                              WatchdogHasPendingSigint);
 
   env->SetMethod(target, "safeGetenv", SafeGetenv);
+
+  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).FromJust();
 }
 
 }  // namespace util