Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: coerce should play well with parser configuration #2308

Merged
merged 7 commits into from Apr 28, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 17 additions & 18 deletions lib/yargs-factory.ts
Expand Up @@ -380,48 +380,47 @@ 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 "coerceKey", 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<Arguments> | Promise<Partial<Arguments>> => {
let aliases: Dictionary<string[]>;
// 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;
}

return maybeAsyncResult<
Partial<Arguments> | Promise<Partial<Arguments>> | any
>(
() => {
aliases = yargs.getAliases();
return value(argv[keys]);
return value(argv[argvKeys[0]]);
},
(result: any): Partial<Arguments> => {
argv[keys] = result;
const stripAliased = yargs
.getInternalMethods()
.getParserConfiguration()['strip-aliased'];
if (aliases[keys] && stripAliased !== true) {
for (const alias of aliases[keys]) {
argv[alias] = result;
}
}
argvKeys.forEach(key => {
argv[key] = result;
});
return argv;
},
(err: Error): Partial<Arguments> | Promise<Partial<Arguments>> => {
throw new YError(err.message);
}
);
},
keys
coerceKey
);
return this;
}
Expand Down
70 changes: 70 additions & 0 deletions test/yargs.cjs
Expand Up @@ -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', () => {
Expand Down