Improve version option property handling

- Only define it when a version has been set - Show as data property in console output
tj · Jul 31, 2023 · a3f0e28 · a3f0e28
1 parent a5afe93
commit a3f0e28
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 10 deletions.
diff --git a/lib/command.js b/lib/command.js
@@ -78,7 +78,12 @@ class Command extends EventEmitter {
     this._helpCommandDescription = 'display help for command';
     this._helpConfiguration = {};
 
-    this._optionValuesProxy = new Proxy(this._optionValues, {
+    // 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, {
+      get: (_, key) => {
+        return this.getOptionValue(key);
+      },
       set: (_, key, value) => {
         this.setOptionValue(key, value);
         return true;
@@ -93,8 +98,19 @@ Option value deletion is not supported`);
 - or Object.defineProperties(),
 - or Reflect.defineProperty().
 Options value configuration is not supported`);
+      },
+      getOwnPropertyDescriptor: (target, key) => {
+        if (this._storeOptionsAsProperties && key === this._versionOptionName) {
+          return {
+            value: target[key],
+            writable: true,
+            configurable: true,
+            enumerable: true
+          };
+        }
+        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.
@@ -835,10 +851,12 @@ Expecting one of '${allowedValues.join("', '")}'`);
     if (Object.keys(this._optionValues).length) {
       throw new Error('call .storeOptionsAsProperties() before setting option values');
     }
-    if (!this._storeOptionsAsProperties && storeAsProperties) {
-      this._defineVersionOptionAsProperty();
-    } else if (this._storeOptionsAsProperties && !storeAsProperties) {
-      this._deleteVersionOptionProperty();
+    if (this._versionOptionName !== undefined) {
+      if (!this._storeOptionsAsProperties && storeAsProperties) {
+        this._defineVersionOptionProperty();
+      } else if (this._storeOptionsAsProperties && !storeAsProperties) {
+        this._deleteVersionOptionProperty();
+      }
     }
     this._storeOptionsAsProperties = !!storeAsProperties;
     return this;
@@ -1889,7 +1907,7 @@ Add support for option by calling .option() or .addOption() first`);
    *
    * You can optionally supply the  flags and description to override the defaults.
    *
-   * @param {string} str
+   * @param {string} [str]
    * @param {string} [flags]
    * @param {string} [description]
    * @return {this | string} `this` command for chaining, or version string if no arguments
@@ -1903,7 +1921,7 @@ Add support for option by calling .option() or .addOption() first`);
     const versionOption = this.createOption(flags, description);
     if (this._storeOptionsAsProperties) this._deleteVersionOptionProperty();
     this._versionOptionName = versionOption.attributeName();
-    if (this._storeOptionsAsProperties) this._defineVersionOptionAsProperty();
+    if (this._storeOptionsAsProperties) this._defineVersionOptionProperty();
     this.options.push(versionOption);
     this.on('option:' + versionOption.name(), () => {
       this._outputConfiguration.writeOut(`${str}\n`);
@@ -1915,7 +1933,7 @@ Add support for option by calling .option() or .addOption() first`);
   /**
    * @api private
    */
-  _defineVersionOptionAsProperty() {
+  _defineVersionOptionProperty() {
     return Reflect.defineProperty(this._optionValues, this._versionOptionName, {
       get: () => this._version,
       set: (value) => {

diff --git a/typings/index.d.ts b/typings/index.d.ts
@@ -293,7 +293,7 @@ export class Command {
    *
    * You can optionally supply the  flags and description to override the defaults.
    */
-  version(str: string, flags?: string, description?: string): this;
+  version(str?: string, flags?: string, description?: string): this;
 
   /**
    * Define a command, implemented using an action handler.