chore(ts): tsify lib/validation

[mleguen]

Follow up of the 2nd step of #1586: this PR tsifies the lib/validation module.

To do so, it also:

• removes some unused or no longer used function parameters

[bcoe]

This continues to look good to me, the one architectural nit I think we should consider, is can we keep the footprint of yargs' number of new files a bit lower (this could simply mean rolling up a few files together).

[bcoe]

one thought that jumped out at me reviewing this PR. Is that, while I like this break down compositionally, i.e., that we're starting to have types for everything, we've significantly increases our footprint on disk (npm pack indicates we drop 150 files on to disk now).

This is enough files that it can start to impact install/import times. I wonder if it might be worth to start with a lib/types/index.ts for simple types like this, only breaking out significant types into their own files.

tldr; I'm just thinking it might be worth being a bit more sparing with the number of files we write to disk on install.

[mleguen]

It is something I did not think about...

I do find it really useful when developing to keep different things separated into different files (easier to have several windows open on different files, etc.)

If this has such an impact on install time, shouldn't we instead minify the compiled code?

[bcoe]

If this has such an impact on install time

I think that if we get in the few hundred file range, which it looks like we're on track for, it will start to impact the install time (at the end of the day npm needs to untar the tarball, and create those files on disk). We should also keep an eye on how much it increases the overall tarball size.

shouldn't we instead minify the compiled code?

This thought had crossed my mind, my concern is that it:

1. complicates our build process.
2. can make introspection a bit harder (not a big deal as long as we have accurate source-maps generated).
3. if we minify will TypeScript do the right thing with the .d.ts files? We want to make sure that folks in an IDE get the benefit of types

---

I think a reasonable compromise might be sticking with your approach of breaking helper functions into classes/sub-files as needed, but it might not be unreasonable to start with a slightly larger index.types.ts file (I noticed types were a lot of the file footprint).

[mleguen]

I noticed types were a lot of the file footprint

Here are some first ideas to develop:

• use typescript-bundle (but I had to check first what it does exactly, and if it is enough maintained)
• use babel with @babel/preset-typescript (but it would complexify our build process, and I am not sure import xxx = require('xxx') is supported, and we still need it to import old style dependencies)
• find a way not to have a typea.js and typea.js.map for each file in types/, when only the type.d.ts is needed
• find a way to bundle types, at least for the types/ directory

[mleguen]

I removed source maps from the pack (no need to generate them when packing as we do not distribute the ts source files).

I also found that we accidentally packed test files too, which I removed too.

This does not solve the footprint issue in itself, but it is a start...

[mleguen]

This could suit our needs of packing a single d.ts file: https://api-extractor.com/pages/overview/intro/

and could also be useful in a second time to keep API documentation up to date.

I will try to have a closer look at it in the coming weeks.

[bcoe]

@mleguen it been my experience distributing bundles that it's nice to have the source-map, because then when a user reports errors to us, we can have them set:

NODE_OPTION=--enable-source-map

And actually see the original line of source that had the exception.

[bcoe]

I suppose I'd rather have 150 files than complicate our build, or not have source-maps ... but I truly do think the better approach might be to not have so many type files.

I was talking to a peer about how we converted our libraries to TypeScript, another potential suggestion was, if the types are meant for external consumption (as I believe is the case for some of our more internal types?) Just have them inlined in the ts files. Then we could be selective about pulling the types that we think users will need out into their own files.... thoughts?

I suppose I'm just not seeing the advantage of having a file per type, weighed against the cost of a huge library footprint on disk.

[mleguen]

@mleguen it been my experience distributing bundles that it's nice to have the source-map

I don't mind, but we would have to pack the source files too for this to work (as I understand it, a source map without sources does not work). It would have to be done either by packing ts files directly, or by inlining them in the source maps, both solutions increasing much our footprint.

Isn't this opposed to your wish of reducing our footprint?

[mleguen]

the types [...] Just have them inlined in the ts files

OK, let's do that

[bcoe]

👍 with the caveat that I think we should consider inlining some of our types that are not used by public APIs (either combining them into one file, or even having the interfaces in the class that uses them).

[mleguen]

@bcoe I am going to rebase and merge this branch, and work next on a PR dedicated to reducing the footprint.

[bcoe]

sounds good @mleguen, I'm really happy with this direction.