Skip to content

Commit a5afe93

Browse files
committed
Consistent opts() return value behavior
When using options-as-properties: - Added getter and setter for value of version option - Limit setOptionValueWithSource() to registered options
1 parent 1b94efb commit a5afe93

File tree

1 file changed

+49
-18
lines changed

1 file changed

+49
-18
lines changed

lib/command.js

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,9 @@ Expecting one of '${allowedValues.join("', '")}'`);
589589
const oname = option.name();
590590
const name = option.attributeName();
591591

592+
// register the option
593+
this.options.push(option);
594+
592595
// store default value
593596
if (option.negate) {
594597
// --no-foo is special and defaults foo to true, unless a --foo option is already defined
@@ -600,9 +603,6 @@ Expecting one of '${allowedValues.join("', '")}'`);
600603
this.setOptionValueWithSource(name, option.defaultValue, 'default');
601604
}
602605

603-
// register the option
604-
this.options.push(option);
605-
606606
// handler for cli and env supplied values
607607
const handleOptionValue = (val, invalidValueMessage, valueSource) => {
608608
// val is null for optional option used without an optional-argument.
@@ -829,10 +829,18 @@ Expecting one of '${allowedValues.join("', '")}'`);
829829
*/
830830

831831
storeOptionsAsProperties(storeAsProperties = true) {
832-
this._storeOptionsAsProperties = !!storeAsProperties;
833832
if (this.options.length) {
834833
throw new Error('call .storeOptionsAsProperties() before adding options');
835834
}
835+
if (Object.keys(this._optionValues).length) {
836+
throw new Error('call .storeOptionsAsProperties() before setting option values');
837+
}
838+
if (!this._storeOptionsAsProperties && storeAsProperties) {
839+
this._defineVersionOptionAsProperty();
840+
} else if (this._storeOptionsAsProperties && !storeAsProperties) {
841+
this._deleteVersionOptionProperty();
842+
}
843+
this._storeOptionsAsProperties = !!storeAsProperties;
836844
return this;
837845
}
838846

@@ -869,6 +877,20 @@ Expecting one of '${allowedValues.join("', '")}'`);
869877
*/
870878

871879
setOptionValueWithSource(key, value, source) {
880+
if (this._storeOptionsAsProperties) {
881+
if (key === this._versionOptionName) {
882+
throw new Error(`Tried to set value of option ${key} reserved for version number.
883+
Set version number by calling .version() instead`);
884+
}
885+
const optionSupported = this.options.some(
886+
option => key === option.attributeName()
887+
);
888+
if (!optionSupported) {
889+
throw new Error(`Tried to set value of not supported option ${key}.
890+
This is not allowed when option values are stored as instance properties.
891+
Add support for option by calling .option() or .addOption() first`);
892+
}
893+
}
872894
this._optionValues[key] = value;
873895
this._optionValueSources[key] = source;
874896
return this;
@@ -1627,20 +1649,6 @@ Expecting one of '${allowedValues.join("', '")}'`);
16271649
* @return {Object}
16281650
*/
16291651
opts() {
1630-
if (this._storeOptionsAsProperties) {
1631-
// Preserve original behaviour so backwards compatible when still using properties
1632-
const result = {};
1633-
const len = this.options.length;
1634-
1635-
for (let i = 0; i < len; i++) {
1636-
const key = this.options[i].attributeName();
1637-
result[key] = key === this._versionOptionName
1638-
? this._version
1639-
: this._optionValues[key];
1640-
}
1641-
return result;
1642-
}
1643-
16441652
return this._optionValuesProxy;
16451653
}
16461654

@@ -1893,7 +1901,9 @@ Expecting one of '${allowedValues.join("', '")}'`);
18931901
flags = flags || '-V, --version';
18941902
description = description || 'output the version number';
18951903
const versionOption = this.createOption(flags, description);
1904+
if (this._storeOptionsAsProperties) this._deleteVersionOptionProperty();
18961905
this._versionOptionName = versionOption.attributeName();
1906+
if (this._storeOptionsAsProperties) this._defineVersionOptionAsProperty();
18971907
this.options.push(versionOption);
18981908
this.on('option:' + versionOption.name(), () => {
18991909
this._outputConfiguration.writeOut(`${str}\n`);
@@ -1902,6 +1912,27 @@ Expecting one of '${allowedValues.join("', '")}'`);
19021912
return this;
19031913
}
19041914

1915+
/**
1916+
* @api private
1917+
*/
1918+
_defineVersionOptionAsProperty() {
1919+
return Reflect.defineProperty(this._optionValues, this._versionOptionName, {
1920+
get: () => this._version,
1921+
set: (value) => {
1922+
this._version = value;
1923+
},
1924+
configurable: true,
1925+
enumerable: true
1926+
});
1927+
}
1928+
1929+
/**
1930+
* @api private
1931+
*/
1932+
_deleteVersionOptionProperty() {
1933+
return Reflect.deleteProperty(this._optionValues, this._versionOptionName);
1934+
}
1935+
19051936
/**
19061937
* Set the description.
19071938
*

0 commit comments

Comments
 (0)