format remains a string when set via ncurc.js

[jftanner]

• I have searched for similar issues

---

Steps to Reproduce

.ncurc.js:

module.exports = {
  interactive: true,
  format: 'group' ,// <-- Where the problem starts
  deep: true // <-- What manifests the problem
};

Dependencies: Not relevant

Steps:

1. Run ncu with the above config.

Current Behavior

When the config is loaded, format: 'group' is loaded and kept as a string. Later, after selecting dependencies, ncu crashes with an error: TypeError: (r.format || []).filter is not a function

I did a little digging and determined that the error comes from runLocal.ts#L251. Since 'group' is a string, it doesn't have a .format method.

This probably hasn't been noticed before because ncu works just fine with format: 'group', so long as deep: true isn't also enabled. Other parts of the code -- such as options.format?.includes('group') on runLocal.ts#L93 -- work largely as expected because strings also happen to have a mostly-compatible .includes method.

Specifying format: ['group'] results in the correct behavior.

Expected Behavior

I would expect one of two behaviors:

1. A validation error on load, stating that format must be an array.
2. Or, automatically convert the string to an array like the CLI does

I believe the latter option is the expected behavior, as the project's own .ncurc.js file uses format: 'group'.

[jftanner]

I think there's a couple options to fix this:

• Add a converter somewhere in getNcuRc.
• Add a converter to initOptions, where runOptions gets converted to options.
• Throw an error in initOptions (though, honestly, that should probably be considered a breaking change.)

I'd be happy to submit a PR implementing any of them, if you've got a preference for which one.
Doing it in getNcuRc probably makes the most sense to me, since it's used in a couple places and is expected to output the same parsed parameter types as commander.

[raineorshine]

Hi, thanks for the thorough bug report. Interesting this slipped by for so long.

Yes, the parse function should always be called on the user input. I think getNcuRc is the right place if the problem only occurs with values from the rc file. A PR is greatly appreciated!

[jftanner]

Sorry for the wait on the PR. It's been a busy week. I think I should have some time tomorrow, though.

[jftanner]

So... I just spent a while fixing the issue by updating how args is populated in getNcuRc.ts. But while I was double-checking the downstream code to make sure I wasn't breaking anything, I noticed this in cli.ts:

  // insert config arguments into command line arguments so they can all be parsed by commander
  const combinedArguments = [...process.argv.slice(0, 2), ...rcArgs, ...programArgs]
  //...
  program.parse(combinedArguments)
  const combinedProgramOpts = program.opts()

Which means that the output of getNcuRc().args does get parsed through commander.
To test it, I commented out my "fix" and ran it. Sure enough, options.format is an array when it's passed into ncu. (Funny how much easier this is with a debugger.)

Which is all to say that my original analysis was wrong. The bug isn't in the CLI, but in runUpgrades where it calls getNcuRc directly, without passing it through commander. ... and I'm still trying to figure out what to do about that.

Edit: I understand what's going on now. Config gets parsed to args and also used by itself. That makes sense. I think I can make the necessary fixes.