yargs / yargs/yargs-parser

Repeated config arguments results in a confusing error

Open
#430 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
519
Forks
129
Avg merge
1h 34m
Merged PRs (30d)
1

Description

I am unsure if this belongs here or over in https://github.com/yargs/yargs instead, but I think it's mostly an issue here in the parser.

In any case, I went and wrote a little CLI tool using nodejs and yargs and in particular its .config() mechanism; so far, so good. However, if I pass --config more than once (e.g. --config a.json --config b.json), it dies with the help message and the confusing report Invalid JSON config file: [ './a.json', './b.json' ]. A little brute force shows that that report arises from https://github.com/yargs/yargs-parser/blob/217aa62906d0bbd40e06d675b8a23df8d49a5ad4/lib/yargs-parser.ts#L688 but is, ultimately, because the call to mixin.resolve on https://github.com/yargs/yargs-parser/blob/217aa62906d0bbd40e06d675b8a23df8d49a5ad4/lib/yargs-parser.ts#L666 is, by default, a call to path.resolve (owing to https://github.com/yargs/yargs-parser/blob/90f970a6482dd4f5b5eb18d38596dd6f02d73edf/lib/index.ts#L38) and path.resolve balks if not given a string as its second argument.

At the very least the error could be more informative (perhaps, "configuration options may be given at most once"), but because it's possible to have multiple .config() directives applied to yargs, and so the machinery tolerates the notion of multiple config files in general, I would prefer to wrap the entirety of https://github.com/yargs/yargs-parser/blob/217aa62906d0bbd40e06d675b8a23df8d49a5ad4/lib/yargs-parser.ts#L664-L689 in a loop to handle the case that configPath is an array.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the repeated --config case, then inspect lib/yargs-parser.ts around lines 664-689 and the default resolver in lib/index.ts around line 38. Determine whether array-valued config paths should be handled or rejected with a clearer message, and verify the resulting behavior with the parser's existing tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js, typescript
Domain
cli
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.