-
Notifications
You must be signed in to change notification settings - Fork 1k
Description
Hi! 👋 Firstly, thanks for your work on this project! 🙂
Today I used patch-package to patch yargs@17.4.1 for the project I'm working on. Here's the issue I was having:
Subsequent calls to .options() merge all the properties, except for description. Earlier descriptions are always overwritten with undefined even if the key is not set in later .options() calls. See in the example below how the existing type is kept, and default is overwritten, but description is also removed.
import yargs from 'yargs'
console.log(
await yargs()
.options({ arr: { desc: 'my arrrrrg', type: 'string', default: 'old' } })
.options({ arr: { default: 'new' } })
.getHelp()
)
/*
* Before patch *
Options:
--help Show help [boolean]
--version Show version number [boolean]
--arr [string] [default: 'new']
* After patch *
Options:
--help Show help [boolean]
--version Show version number [boolean]
--arr my arrrrrg [string] [default: 'new']
*/Here is the diff that solved my problem:
diff --git a/node_modules/yargs/build/lib/yargs-factory.js b/node_modules/yargs/build/lib/yargs-factory.js
index f79ad50..2400277 100644
--- a/node_modules/yargs/build/lib/yargs-factory.js
+++ b/node_modules/yargs/build/lib/yargs-factory.js
@@ -667,7 +667,9 @@ export class YargsInstance {
this.skipValidation(key);
}
const desc = opt.describe || opt.description || opt.desc;
- this.describe(key, desc);
+ if (typeof desc === 'string') {
+ this.describe(key, desc);
+ }
if (opt.hidden) {
this.hide(key);
}I couldn't tell if this behavior was intentional or not. All other option properties are guarded in a similar manner, so I thought maybe it was intentional that description was not. But it does seem like a bug to me, or at least I was surprised by this differing behavior.