add tests for setter behaviour

nodejs · May 9, 2018 · fd4d955 · fd4d955
1 parent 5dff49a
commit fd4d955
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 9 deletions.
diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js
@@ -46,14 +46,16 @@
     has: ReflectHas,
     get: ReflectGet,
     set: ReflectSet,
-    defineProperty: ReflectDefineProperty,
     deleteProperty: ReflectDeleteProperty,
     getOwnPropertyDescriptor: ReflectGetOwnPropertyDescriptor,
   } = Reflect;
   const {
-    toString: ObjectToString,
-    hasOwnProperty: ObjectHasOwnProperty,
-  } = Object.prototype;
+    prototype: {
+      toString: ObjectToString,
+      hasOwnProperty: ObjectHasOwnProperty,
+    },
+    defineProperty: ObjectDefineProperty,
+  } = Object;
 
   // Set up process.moduleLoadList
   const moduleLoadList = [];
@@ -301,11 +303,11 @@
               if (typeof value === 'function')
                 descriptor.value = methodUnwrapMap.get(value) || value;
             }
-            if (ReflectDefineProperty(target, prop, descriptor)) {
-              update(prop, ReflectGet(target, prop));
-              return true;
-            }
-            return false;
+            // Use `Object.defineProperty` instead of `Reflect.defineProperty`
+            // to throw the appropriate error if something goes wrong.
+            ObjectDefineProperty(target, prop, descriptor);
+            update(prop, ReflectGet(target, prop));
+            return true;
           },
           deleteProperty: (target, prop) => {
             if (ReflectDeleteProperty(target, prop)) {

diff --git a/test/es-module/test-esm-live-binding.mjs b/test/es-module/test-esm-live-binding.mjs
@@ -4,6 +4,8 @@ import '../common';
 import assert from 'assert';
 
 import fs, { readFile } from 'fs';
+import events from 'events';
+import util from 'util';
 
 const s = Symbol();
 const fn = () => s;
@@ -40,3 +42,58 @@ Reflect.defineProperty(fs, 'readFile', {
 
 assert.strictEqual(fs.readFile(), s);
 assert.strictEqual(readFile(), s);
+
+assert.throws(() => {
+  Object.defineProperty(events, 'defaultMaxListeners', { value: 3 });
+}, {});
+
+// keep these ones last because they mess with prototypes
+
+let count = 0;
+Reflect.defineProperty(Function.prototype, 'defaultMaxListeners', {
+  configurable: true,
+  enumerable: true,
+  get: function() { return ++count; },
+  set: function(v) {
+    Reflect.defineProperty(this, 'defaultMaxListeners', {
+      configurable: true,
+      enumerable: true,
+      writable: true,
+      value: v,
+    });
+  },
+});
+
+assert.strictEqual(10, events.defaultMaxListeners);
+assert.strictEqual(11, ++events.defaultMaxListeners);
+
+assert.strictEqual(1, Function.prototype.defaultMaxListeners);
+assert.strictEqual(2, Function.prototype.defaultMaxListeners);
+Function.prototype.defaultMaxListeners = 'foo';
+assert.strictEqual('foo', Function.prototype.defaultMaxListeners);
+
+assert.strictEqual(11, events.defaultMaxListeners);
+
+count = 0;
+const p = {
+  get foo() { return ++count; },
+  set foo(v) {
+    Reflect.defineProperty(this, 'foo', {
+      configurable: true,
+      enumerable: true,
+      writable: true,
+      value: v,
+    });
+  },
+};
+
+util.__proto__ = p; // eslint-disable-line no-proto
+
+assert.strictEqual(1, util.foo);
+util.foo = 'bar';
+assert.strictEqual(1, count);
+assert.strictEqual('bar', util.foo);
+assert.strictEqual(2, p.foo);
+assert.strictEqual(3, p.foo);
+p.foo = 'foo';
+assert.strictEqual('foo', p.foo);