TypeScript types

[bakkot]

I have a prototype implementation of types for this package, with some type-level shenanigans so that you get exact types on the result as long as you pass the config directly to parseArgs. E.g.:

let { values, positionals } = parseArgs({
 options: {
 foo: { type: 'string', multiple: true },
 bar: { type: 'boolean' },
 evil: { type: 'string', multiple: Math.random() < 0.5 },
 },
});
let foo: string[] = values.foo; // works
let bar: boolean | undefined = values.bar; // works
let evil: string[] | string | undefined = values.evil; // if we can't precisely infer `multiple`, we can't say if it's an array or not
positionals[0]; // error: since we did not explicitly specify `allowPositionals: true`, the `positionals` array must be empty

Any feedback before I submit them upstream? You can play around with the types in the playground linked, no need to install anything.

[shadowspawn]

The magic to make it work is a bit opaque to me, but the results look good from experimenting. The strict: false ones are annoyingly inexact, but look accurate! And careful callers will need to cope.

[bcoe]

@bakkot the types seem solid to me, I appreciate that there are useful affordances, e.g., having typing for arrays, having the array default to empty if allowPositionals not set.

@bakkot could we publish the types to both to node:util and to pkgjs/parseargs in DefinitelyTyped, alternatively is there an easy way to for us to reference the node:util types when using parseArgs, and we can document this in the README?

[ljharb]

There’s no easy way in DT for one package to reference the types for another, so the best approach i think is to duplicate them in both DT packages.

[bakkot]

@bcoe Since you control parseArgs, you can include the types for it in the package itself rather than putting them on DefinitelyTyped. And in that case, yes, you can re-export the types for the version of parseArgs in node:util from DefinitelyTyped, once those exist.

That said, it looks like types for 18.0.0 are still WIP, so I'm probably not going to submit these for util.parseArgs for a while yet. Maybe it makes sense to just provide them for pkgjs/parseargs for now? And in that case, would you be OK with having them live in this repo instead of DefinitelyTyped for the moment?

[ljharb]

I find it vastly superior for types to be in DT instead of the package directly, since it avoids conflating the semver of the implementation with that of the types.

[shadowspawn]

My comments are at a non-expert level.

To keep the version numbers sane and allow parseArgs package to include changes which have not yet landed in node, I was expecting separate type definitions (even it matching) for parseArgs package and node:util. Rather than one referencing the other.

As for whether to include in package, a comment each way!

I'm not concerned about the conflation of types and code by publishing within package. I consider the types part of the package.

However, the TypeScript publishing guidance, https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#handbook-content says:

If your types are generated by your source code, publish the types with your source code. Both TypeScript and JavaScript projects can generate types via declaration.

Otherwise, we recommend submitting the types to DefinitelyTyped, which will publish them to the @types organization on npm.

[aaronccasanova]

Nice work @bakkot!! I copied your TS Playground and included some light suggestions, specifically:

• Copy/pasted the Exact and IsEqual utility types from type-fest
  • Use Exact to error on unknown config properties
  • Use IsEqual to infer various boolean configs e.g. IsStrict, IsAllowPositionals, IsMultiple
• Restructured all type conditions to avoid negated types (e.g. IsStrict extends true : true-branch : false-branch)
• Tentatively renamed inference types as it is common to type import them in scripts and imo would be a good idea to establish a solid naming convention

Additionally, I added a question on the first usage example where:
config.strict === true && optionConfig.multiple === true

The values type infers values.foo as foo: string[]. However, if the user never passes --foo then values.foo would be undefined IIRC.

[bakkot]

@aaronccasanova Neat! Comments:

---

Re: "Shouldn't foo values also have undefined"

Uhh I guess you're right. I assumed that multiple options would be present as an empty array if not passed, but that doesn't seem to be the case.

---

Your changes have broken some of the types. Specifically, evil is now inferred as string | undefined, whereas the correct type is string[] | string | undefined (because the point of the evil property is that we don't know if it's multiple or not). The ExtractIsStrict thing you're doing is meaningfully different from the IfDefaultsTrue and IfDefaultsFalse helpers I had, which handle true, false, and boolean (i.e. present but not known to be either true or false) as distinct cases.

[aaronccasanova]

I assumed that multiple options would be present as an empty array if not passed, but that doesn't seem to be the case.

Yeah, we don't traverse the config and set defaults on the parsed results. The update is simply adding undefined to the true-branch of IsMultiple extends true:

Specifically, evil is now inferred as string | undefined, whereas the correct type is string[] | string | undefined

I'm seeing the correct types.

The ExtractIsStrict thing you're doing is meaningfully different from the IfDefaultsTrue and IfDefaultsFalse helpers I had, which handle true, false, and boolean (i.e. present but not known to be either true or false) as distinct cases.

We can confidently infer strict since we default to true and validate the input is a boolean. My thinking was we only have to check is if the input is explicitly false but perhaps there are holes in that logic.

[bakkot]

I'm seeing the correct types.

That's because you're looking at a variable which is explicitly annotated with that type. Look at the type for values.evil (or remove the string[] from the type annotation and see that it still checks).

We can confidently infer strict since we default to true and validate the input is a boolean. My thinking was we only have to check is if the input is explicitly false but perhaps there are holes in that logic.

The hole is, knowing that it is definitely a boolean and is not known to be true does not tell you it's definitely false. The type system might just not be able to tell which it is.

[aaronccasanova]

Oh I see! Hmm now I'm wondering if that applies to all the Extract default utilities in my prototype.

[bakkot]

For strict and allowPositionals it doesn't end up mattering because the types for "not known to be true" are the same as the types for "known to be false", but the distinction is relevant for multiple.

[aaronccasanova]

Welp, I totally understand the rationale for IfDefaultsTrue and IfDefaultsFalse now! 👍 I love the composition in ParsedPositionals:

I tested every permutation and it's very robust 👏

Here is an updated prototype reverting the IsEqual/Extract utilties. This version includes:

• Updated naming conventions
• Errors for unknown config properties
• Proposed type fix for values.foo when the option is not provided to the CLI

As mentioned above, these are light suggestions so feel free to dismiss 🙂

[bakkot]

Nice, LGTM.