If you're not familiar with the boolean trap, here is a short summary extracted from the article:

Let’s start with the textbook example: guess what this code means?

widget.repaint(false);

Without looking at the documentation, the usual suspect is don’t repaint the widget. Then, you look at the documentation and it refers the function argument as immediate, which is true if you want immediate painting or false for deferred painting. Thus, the correct behavior implied by the above code is actually repaint this widget later, which is miles away from your initial guess.

We have a few place where we fall into the boolean trap. For example, without looking, can you tell without any doubt the signification of the boolean arguments in the following places?

await setVersion(config, reporter, flags, args, true);
await buildTree(this.resolver, this.linker, patterns, opts, true, true);
await ls(manifest, this.reporter, true);
await this.fork(GitResolver, true, `${sshUrl}#${commit}`);
...

Note that the last one is actually a double trap: this true isn't actually an option, it's an argument that is then passed to the Git resolver. Ideally, these examples could be reworded as such:

await setVersion(config, reporter, flags, args, {isRequired: true});
await buildTree(this.resolver, this.linker, patterns, opts, {onlyFresh: true, ignoreHoisted: true});
await ls(manifest, this.reporter, {isSaved: true});
await this.fork(GitResolver, { forked: true }, `${sshUrl}#${commit}`);
...

It would be a good first step. However, I suggest we go further: let's move into the trailing object all the parameters that have strictly no semantic meaning in the context of the function. For example, consider the following function call:

await setVersion(config, reporter, flags, args, true);

Here, the flags and args parameters have a semantic meaning. We are calling the setVersion command, which takes options and arguments, so it makes sense for them to be first-class arguments. However, everything else (config, reporter and isRequired) is required only because of implementation details and doesn't help to actually understand the code flow.

Because of this, I suggest we write the following instead:

await setVersion(flags, args, {config, reporter, isRequired: true});

The example above would then become:

await setVersion(flags, args, {config, reporter, isRequired: true});
await buildTree(patterns, {resolver: this.resolver, link: this.linker, onlyFresh: true, ignoreHoisted: true, reqDepth: ... });
await ls(manifest, {reporter: this.reporter, isSaved: true});
...

Is it required?

No, it's just a stylistic proposal that I think could improve the readability. If you think this is nonsense we can drop it :)

Would we have to rewrite all the methods?

No, it's something that would only affect the new code we write and potentially the existing code we would need to rework anyway while implementing features and/or fixing bugs. There's no need to update everything at once.

Would it work with prettier?

I think so. It would follow the classic conventions: until 80 columns it would stay on the same line, and more than that it would go multiline. I think there is some room of improvement on the prettier/Flow interaction on the object typing (more on that below), but overall there's no blocker.

Would it work with Flow?

There's a bit more work because Flow has a somewhat particular syntax to declare argument objects, partly because of the JS spec preventing them to use the same syntax as everywhere else (because { config: Config } means "assign config to the variable Config in destructuring contexts).

So, instead of writing:

async function setVersion(
  config: Config,
  reporter: Reporter,
  flags: Object,
  args: Array<string>,
  isRequired: ?boolean
) {}

We would write:

async function setVersion(
  flags: Object,
  args: Array<string>,
  {
    config,
    reporter,
    boolean = false
  }: { config: Config, reporter: Reporter, isRequired: boolean }
) {}

I would personally prefer Prettier to mirror the style regardless of the width, so that it would become:

async function setVersion(
  flags: Object,
  args: Array<string>,
  {
    config,
    reporter,
    boolean = false
  }: {
    config: Config,
    reporter: Reporter,
    isRequired: boolean
  }
) {}

But that's a good first step regardless.

Opinions?