Return proxy from constructor further up the prototype chain

The change ensures the this object used in methods is the proxy from the very beginning. This solves pretty much all issues with returning the proxy from a constructor. For example, private fields can be used now. More details available at: tj#1921 (comment) Additionally, wrong spelling has been fixed in comments.
aweebit · Aug 1, 2023 · eb142d8 · eb142d8
1 parent a3f0e28
commit eb142d8
Showing 1 changed file with 69 additions and 62 deletions.
diff --git a/lib/command.js b/lib/command.js
@@ -12,7 +12,69 @@ const { suggestSimilar } = require('./suggestSimilar');
 
 // @ts-check
 
-class Command extends EventEmitter {
+class CommandBase extends EventEmitter {
+  constructor() {
+    super();
+
+    // The proxy only treats keys not present in the instance and its prototype chain as keys for _optionValues when _storeOptionsAsProperties is set to true.
+    // Setting option values for keys present in the instance and its prototype chain is still possible by calling .setOptionValue() or .setOptionValueWithSource(),
+    // but such values will not be accessible as instance properties because the instance and its prototype chain have precedence.
+    // However, they will be accessible via .getOptionValue(), .opts() and .optsWithGlobals().
+    return new Proxy(this, {
+      get(target, key, receiver) {
+        if (target._storeOptionsAsProperties && !(key in target)) {
+          target = receiver = receiver._optionValuesProxy;
+        }
+        return Reflect.get(target, key, receiver);
+      },
+      set(target, key, value, receiver) {
+        if (target._storeOptionsAsProperties && !(key in target)) {
+          target = receiver = receiver._optionValuesProxy;
+        }
+        return Reflect.set(target, key, value, receiver);
+      },
+      has(target, key) {
+        if (target._storeOptionsAsProperties && !(key in target)) {
+          target = target._optionValuesProxy;
+        }
+        return Reflect.has(target, key);
+      },
+      deleteProperty(target, key) {
+        if (target._storeOptionsAsProperties && !(key in target)) {
+          target = target._optionValuesProxy;
+        }
+        return Reflect.deleteProperty(target, key);
+      },
+      defineProperty(target, key, descriptor) {
+        if (target._storeOptionsAsProperties && !(key in target)) {
+          target = target._optionValuesProxy;
+        }
+        return Reflect.defineProperty(target, key, descriptor);
+      },
+      getOwnPropertyDescriptor(target, key) {
+        if (target._storeOptionsAsProperties && !(key in target)) {
+          target = target._optionValuesProxy;
+        }
+        return Reflect.getOwnPropertyDescriptor(target, key);
+      },
+      ownKeys(target) {
+        const result = Reflect.ownKeys(target);
+        if (target._storeOptionsAsProperties) {
+          result.push(...Reflect.ownKeys(target._optionValuesProxy));
+        }
+        return result;
+      },
+      preventExtensions(target) {
+        if (target._storeOptionsAsProperties) {
+          Reflect.preventExtensions(target._optionValuesProxy);
+        }
+        return Reflect.preventExtensions(target);
+      }
+    });
+  }
+}
+
+class Command extends CommandBase {
   /**
    * Initialize a new `Command`.
    *
@@ -78,6 +140,12 @@ class Command extends EventEmitter {
     this._helpCommandDescription = 'display help for command';
     this._helpConfiguration = {};
 
+    // Because of how the proxy returned from the CommandBase constructor works in order to support options-as-properties,
+    // all instance properties have to be defined when _storeOptionsAsProperties is set to false.
+    // Ideally, that should happen as soon as in the constructor, even if it seems unnecessary because the initial values are undefined like here.
+    this._version = undefined;
+    this._versionOptionName = undefined;
+
     // Double proxy to show the version option property value instead of [Getter/Setter] when printing the return value of opts() to a console.
     // Required because Node internally unwraps one proxy and therefore would not use the getOwnPropertyDescriptor() trap otherwise.
     this._optionValuesProxy = new Proxy(new Proxy(this._optionValues, {
@@ -111,67 +179,6 @@ Options value configuration is not supported`);
         return Reflect.getOwnPropertyDescriptor(target, key);
       }
     }), {});
-
-    // Because of how the returned proxy works, ideally, no prooerties should be defined outside the cinstructor.
-    // They can still be defined outside the constructor in subclasses, but only when _storeOptionsAsProperties is set to false.
-    this._version = undefined;
-    this._versionOptionName = undefined;
-
-    // The proxy only treats keys not present in the instance and its prototype chain as keys for _optionValues when _storeOptionsAsProperties is set to true.
-    // Setting option values for keys present in the instance and its prototype chain is still possible by calling .setOptionValue() or .setOptionValueWithSource(),
-    // but such values will not be accessible as instnace properties because the instance and its prototype chain has precedence.
-    // However, they will be accessible via .getOptionValue(), .opts() and .optsWithGlobals().
-    return new Proxy(this, {
-      get(target, key, receiver) {
-        if (target._storeOptionsAsProperties && !(key in target)) {
-          target = receiver = receiver._optionValuesProxy;
-        }
-        return Reflect.get(target, key, receiver);
-      },
-      set(target, key, value, receiver) {
-        if (target._storeOptionsAsProperties && !(key in target)) {
-          target = receiver = receiver._optionValuesProxy;
-        }
-        return Reflect.set(target, key, value, receiver);
-      },
-      has(target, key) {
-        if (target._storeOptionsAsProperties && !(key in target)) {
-          target = target._optionValuesProxy;
-        }
-        return Reflect.has(target, key);
-      },
-      deleteProperty(target, key) {
-        if (target._storeOptionsAsProperties && !(key in target)) {
-          target = target._optionValuesProxy;
-        }
-        return Reflect.deleteProperty(target, key);
-      },
-      defineProperty(target, key, descriptor) {
-        if (target._storeOptionsAsProperties && !(key in target)) {
-          target = target._optionValuesProxy;
-        }
-        return Reflect.defineProperty(target, key, descriptor);
-      },
-      getOwnPropertyDescriptor(target, key) {
-        if (target._storeOptionsAsProperties && !(key in target)) {
-          target = target._optionValuesProxy;
-        }
-        return Reflect.getOwnPropertyDescriptor(target, key);
-      },
-      ownKeys(target) {
-        const result = Reflect.ownKeys(target);
-        if (target._storeOptionsAsProperties) {
-          result.push(...Reflect.ownKeys(target._optionValuesProxy));
-        }
-        return result;
-      },
-      preventExtensions(target) {
-        if (target._storeOptionsAsProperties) {
-          Reflect.preventExtensions(target._optionValuesProxy);
-        }
-        return Reflect.preventExtensions(target);
-      }
-    });
   }
 
   /**