chakrashim: implement proxy methods in chakrashim

Implement value::IsProxy(), Proxy::GetTarget() and Proxy::GetHandler() in chakrashim. This enables util.types.isProxy() and util.inspect() with the showProxy flag as true to function correctly in node-chakracore. Also enable related tests for node-chakracore: * parallel/test-util-types.js * parallel/test-util-inspect.js Fixes: nodejs#488 Fixes: nodejs#523 PR-URL: nodejs#525 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jack Horton <Jack.Horton@microsoft.com>
kfarnung · May 10, 2018 · e04616c · e04616c
1 parent 2cd0485
commit e04616c
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 24 deletions.
diff --git a/deps/chakrashim/lib/chakra_shim.js b/deps/chakrashim/lib/chakra_shim.js
@@ -636,11 +636,6 @@
       return microTasks.shift();
     };
 
-    utils.isProxy = function(value) {
-      // CHAKRA-TODO: Need to add JSRT API to detect this
-      return false;
-    };
-
     utils.getPropertyAttributes = function(object, value) {
       const descriptor = Object_getOwnPropertyDescriptor(object, value);
       if (descriptor === undefined) {

diff --git a/deps/chakrashim/src/v8proxy.cc b/deps/chakrashim/src/v8proxy.cc
@@ -22,15 +22,23 @@
 
 namespace v8 {
 Local<Object> Proxy::GetTarget() {
-  // CHAKRA-TODO: Need to add JSRT API to get target of proxy
-  JsValueRef undefinedValue = jsrt::GetUndefined();
-  return Local<Object>::New(undefinedValue);
+  JsValueRef returnValue = JS_INVALID_REFERENCE;
+  bool isProxy = false;
+  JsGetProxyProperties((JsValueRef) this, &isProxy, &returnValue, nullptr);
+  if (!isProxy || returnValue == JS_INVALID_REFERENCE) {
+    returnValue = jsrt::GetUndefined();
+  }
+  return Local<Object>::New(returnValue);
 }
 
 Local<Value> Proxy::GetHandler() {
-  // CHAKRA-TODO: Need to add JSRT API to get handler of proxy
-  JsValueRef undefinedValue = jsrt::GetUndefined();
-  return Local<Value>::New(undefinedValue);
+  JsValueRef returnValue = JS_INVALID_REFERENCE;
+  bool isProxy = false;
+  JsGetProxyProperties((JsValueRef) this, &isProxy, nullptr, &returnValue);
+  if (!isProxy || returnValue == JS_INVALID_REFERENCE) {
+    returnValue = jsrt::GetUndefined();
+  }
+  return Local<Object>::New(returnValue);
 }
 
 Proxy *Proxy::Cast(v8::Value *obj) {

diff --git a/deps/chakrashim/src/v8value.cc b/deps/chakrashim/src/v8value.cc
@@ -145,6 +145,12 @@ bool Value::IsUint32() const {
   return trunc(value) == value;
 }
 
+bool Value::IsProxy() const {
+  bool isProxy = false;
+  JsGetProxyProperties((JsValueRef) this, &isProxy, nullptr, nullptr);
+  return isProxy;
+}
+
 #define IS_TYPE_FUNCTION(v8ValueFunc, chakrashimFunc) \
 bool Value::v8ValueFunc() const { \
 JsValueRef resultRef = JS_INVALID_REFERENCE; \
@@ -163,7 +169,6 @@ IS_TYPE_FUNCTION(IsDate, isDate)
 IS_TYPE_FUNCTION(IsMap, isMap)
 IS_TYPE_FUNCTION(IsNativeError, isNativeError)
 IS_TYPE_FUNCTION(IsPromise, isPromise)
-IS_TYPE_FUNCTION(IsProxy, isProxy)
 IS_TYPE_FUNCTION(IsRegExp, isRegExp)
 IS_TYPE_FUNCTION(IsAsyncFunction, isAsyncFunction)
 IS_TYPE_FUNCTION(IsSet, isSet)

diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status
@@ -86,7 +86,6 @@ test-trace-events-v8 : SKIP
 test-url-domain-ascii-unicode : SKIP
 test-util : SKIP
 test-util-format-shared-arraybuffer : SKIP
-test-util-inspect-proxy : SKIP
 test-v8-serdes : SKIP
 test-vm-attributes-property-not-on-sandbox : SKIP
 test-vm-cached-data : SKIP
@@ -156,7 +155,6 @@ test-vm-module-errors : SKIP
 test-vm-module-import-meta : SKIP
 test-vm-module-link : SKIP
 test-vm-module-reevaluate : SKIP
-test-util-types : SKIP
 
 # This test requires support for bigints
 test-util-inspect-bigint : SKIP

diff --git a/test/parallel/test-util-types.js b/test/parallel/test-util-types.js
@@ -51,9 +51,7 @@ for (const [ value, _method ] of [
                           { value: 'foo' }) ],
   [ new DataView(new ArrayBuffer()) ],
   [ new SharedArrayBuffer() ],
-  // Node-ChakraCore does not support util.types.isProxy() due to
-  // Microsoft/ChakraCore#950 and node/node-chakracore#488
-  !common.isChakraEngine ? [ new Proxy({}, {}), 'isProxy' ] : [ new Date() ],
+  [ new Proxy({}, {}), 'isProxy' ],
   [ new WebAssembly.Module(wasmBuffer), 'isWebAssemblyCompiledModule' ],
 ]) {
   const method = _method || `is${value.constructor.name}`;
@@ -130,10 +128,13 @@ for (const [ value, _method ] of [
   }
 }
 
-(async () => {
-  const m = new vm.Module('');
-  await m.link(() => 0);
-  m.instantiate();
-  await m.evaluate();
-  assert.ok(types.isModuleNamespaceObject(m.namespace));
-})();
+// Node-ChakraCore does not support esmodules
+if (!common.isChakraEngine) {
+  (async () => {
+    const m = new vm.Module('');
+    await m.link(() => 0);
+    m.instantiate();
+    await m.evaluate();
+    assert.ok(types.isModuleNamespaceObject(m.namespace));
+  })();
+}