From 2bb572f0eba8eb2b341ed9ae0a6213c6b97bbde3 Mon Sep 17 00:00:00 2001 From: John Gee Date: Fri, 10 Mar 2023 22:16:22 +1300 Subject: [PATCH 1/4] fix: coerce should play well with parser configuration --- lib/yargs-factory.ts | 10 +++++----- test/yargs.cjs | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 2bfc5224a..ba4175533 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -406,12 +406,12 @@ export class YargsInstance { }, (result: any): Partial => { argv[keys] = result; - const stripAliased = yargs - .getInternalMethods() - .getParserConfiguration()['strip-aliased']; - if (aliases[keys] && stripAliased !== true) { + // yargs-parser takes into account the parser options like strip-aliased and strip-dashed and camel-case expansion. + // Update any naming variations returned by yargs-parser with the coerced value. + if (aliases[keys]) { for (const alias of aliases[keys]) { - argv[alias] = result; + if (Object.prototype.hasOwnProperty.call(argv, alias)) + argv[alias] = result; } } return argv; diff --git a/test/yargs.cjs b/test/yargs.cjs index 42b4b83b8..dffb8d0fc 100644 --- a/test/yargs.cjs +++ b/test/yargs.cjs @@ -2434,6 +2434,22 @@ describe('yargs dsl tests', () => { argv.doGooder.should.equal('BATMAN'); }); + // Addresses: https://github.com/yargs/yargs/issues/2307 + it('when use strip-aliased then coerce should still work with camelcase', () => { + const argv = yargs('--max-size 1') + .option('max-size', { + type: 'number', + coerce: n => n + 100, + alias: 'max-alias', + }) + .parserConfiguration({'strip-aliased': true}) + .parse(); + argv['max-size'].should.equal(101); // coerce original + argv['maxSize'].should.equal(101); // coerce camel-case + expect(argv['max-alias']).to.equal(undefined); // strip-alias + expect(argv['maxAlias']).to.equal(undefined); // strip-alias + }); + it('allows a boolean type to be specified', () => { const argv = yargs('cmd false') .command('cmd [run]', 'a command', yargs => { From a492e88c7e5528c727ab79756d8c926a5c3dd7ca Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 11 Mar 2023 12:11:18 +1300 Subject: [PATCH 2/4] fix: rework coerce to remove assumptions about what keys are present in argv --- lib/yargs-factory.ts | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index ba4175533..7aa811791 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -380,20 +380,26 @@ export class YargsInstance { if (!value) { throw new YError('coerce callback must be provided'); } + + // Handled multiple above, down to one key. + const coerceKey = keys; // This noop tells yargs-parser about the existence of the option - // represented by "keys", so that it can apply camel case expansion + // represented by "key", so that it can apply camel case expansion // if needed: - this.#options.key[keys] = true; + this.#options.key[coerceKey] = true; this.#globalMiddleware.addCoerceMiddleware( ( argv: Arguments, yargs: YargsInstance ): Partial | Promise> => { - let aliases: Dictionary; + // Narrow down the possible keys to the ones present in argv. + const coerceKeyAliases = yargs.getAliases()[coerceKey] ?? []; + const argvKeys = [coerceKey, ...coerceKeyAliases].filter(key => + Object.prototype.hasOwnProperty.call(argv, key) + ); - // Skip coerce logic if related arg was not provided - const shouldCoerce = Object.prototype.hasOwnProperty.call(argv, keys); - if (!shouldCoerce) { + // Skip coerce if nothing to coerce. + if (argvKeys.length === 0) { return argv; } @@ -401,19 +407,12 @@ export class YargsInstance { Partial | Promise> | any >( () => { - aliases = yargs.getAliases(); - return value(argv[keys]); + return value(argv[argvKeys[0]]); }, (result: any): Partial => { - argv[keys] = result; - // yargs-parser takes into account the parser options like strip-aliased and strip-dashed and camel-case expansion. - // Update any naming variations returned by yargs-parser with the coerced value. - if (aliases[keys]) { - for (const alias of aliases[keys]) { - if (Object.prototype.hasOwnProperty.call(argv, alias)) - argv[alias] = result; - } - } + argvKeys.forEach(key => { + argv[key] = result; + }); return argv; }, (err: Error): Partial | Promise> => { @@ -421,7 +420,7 @@ export class YargsInstance { } ); }, - keys + coerceKey ); return this; } From a59be5e5cc192b8a41f7a8c946bbfdc7cfc34ffb Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 11 Mar 2023 12:13:16 +1300 Subject: [PATCH 3/4] chore: fix comment --- lib/yargs-factory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 7aa811791..a7c77f564 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -384,7 +384,7 @@ export class YargsInstance { // Handled multiple above, down to one key. const coerceKey = keys; // This noop tells yargs-parser about the existence of the option - // represented by "key", so that it can apply camel case expansion + // represented by "coerceKey", so that it can apply camel case expansion // if needed: this.#options.key[coerceKey] = true; this.#globalMiddleware.addCoerceMiddleware( From 03b9ce61bc0d55631594419f1c9bc2db2f60a455 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 11 Mar 2023 14:03:57 +1300 Subject: [PATCH 4/4] chore: expand coerce tests --- test/yargs.cjs | 86 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 70 insertions(+), 16 deletions(-) diff --git a/test/yargs.cjs b/test/yargs.cjs index dffb8d0fc..2011a8288 100644 --- a/test/yargs.cjs +++ b/test/yargs.cjs @@ -2238,6 +2238,76 @@ describe('yargs dsl tests', () => { .getHelp(); help.should.match(/option2 description/); }); + + it('argv includes coerced aliases', () => { + const argv = yargs('--foo bar') + .option('foo', { + coerce: s => s.toUpperCase(), + alias: 'f', + }) + .parse(); + argv['foo'].should.equal('BAR'); + argv['f'].should.equal('BAR'); + }); + + it('argv includes coerced camelCase', () => { + const argv = yargs('--foo-foo bar') + .option('foo-foo', { + coerce: s => s.toUpperCase(), + }) + .parse(); + argv['foo-foo'].should.equal('BAR'); + argv['fooFoo'].should.equal('BAR'); + }); + + it('coerce still works when key used for coerce is not explicitly present in argv', () => { + const argv = yargs('--foo-foo bar') + .option('foo-foo') + .coerce('foo-foo', s => s.toUpperCase()) + .parserConfiguration({'strip-dashed': true}) + .parse(); + expect(argv['foo-foo']).to.equal(undefined); + argv['fooFoo'].should.equal('BAR'); + }); + + it('argv does not include stripped aliases', () => { + const argv = yargs('-f bar') + .option('foo-foo', { + coerce: s => s.toUpperCase(), + alias: 'f', + }) + .parserConfiguration({'strip-aliased': true}) + .parse(); + argv['foo-foo'].should.equal('BAR'); + argv['fooFoo'].should.equal('BAR'); + expect(argv['f']).to.equal(undefined); + }); + + it('argv does not include stripped dashes', () => { + const argv = yargs('-f bar') + .option('foo-foo', { + coerce: s => s.toUpperCase(), + alias: 'f', + }) + .parserConfiguration({'strip-dashed': true}) + .parse(); + expect(argv['foo-foo']).to.equal(undefined); + argv['fooFoo'].should.equal('BAR'); + argv['f'].should.equal('BAR'); + }); + + it('argv does not include disabled camel-case-expansion', () => { + const argv = yargs('-f bar') + .option('foo-foo', { + coerce: s => s.toUpperCase(), + alias: 'f', + }) + .parserConfiguration({'camel-case-expansion': false}) + .parse(); + argv['foo-foo'].should.equal('BAR'); + expect(argv['fooFoo']).to.equal(undefined); + argv['f'].should.equal('BAR'); + }); }); describe('stop parsing', () => { @@ -2434,22 +2504,6 @@ describe('yargs dsl tests', () => { argv.doGooder.should.equal('BATMAN'); }); - // Addresses: https://github.com/yargs/yargs/issues/2307 - it('when use strip-aliased then coerce should still work with camelcase', () => { - const argv = yargs('--max-size 1') - .option('max-size', { - type: 'number', - coerce: n => n + 100, - alias: 'max-alias', - }) - .parserConfiguration({'strip-aliased': true}) - .parse(); - argv['max-size'].should.equal(101); // coerce original - argv['maxSize'].should.equal(101); // coerce camel-case - expect(argv['max-alias']).to.equal(undefined); // strip-alias - expect(argv['maxAlias']).to.equal(undefined); // strip-alias - }); - it('allows a boolean type to be specified', () => { const argv = yargs('cmd false') .command('cmd [run]', 'a command', yargs => {