Pull Request Test Coverage Report for Build 223

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+6.4%) to 79.366%

Totals
Change from base Build 220:	6.4%
Covered Lines:	744
Relevant Lines:	892

💛 - Coveralls

It is always a good idea to raise an issue before raising a PR. Otherwise, it doesn't feel nice to just shutdown good PRs, and it definitely does not make someone to contribute again.

In this particular case, I don't see the benefits named (bundle size) as being of great benefit considering this package is for Node.js consumption. The require time does not change in any meaningful way.

Please don’t worry about deterring me. I know this kinda PR is a significant change and not always something the maintainer wants to consider.

I agree with you on the bundle size but have you considered the gain of removing ajv as a dependency that I mentioned? You can find an implementation in this commit: 0d3fe98

The bigger question is "why".

If this was a package that was ever included in browser, I'd be Okay with it.

But since it is only ever used in CLI, and since (when gzipped) all the JavaScript files are tiny, it makes little to no difference to anyone who consumes the package.

I generally follow the KISS principle until someone can demonstrate something that is either visibly breaking or limits them from using the package.

In this particular case, the only use case for rollup that I can see would be to make this package compatible with Deno. I am not an expert on Deno, though.

However, all of this would come at a cost of debug-ability + the overall size of your dependency tree would (very likely) be greater after these changes, because you are bundling dependencies that can no longer be deduplicated.

I've no experience with Deno so I can't comment on that part.

I do think there's an inherent value in reducing the number of dependencies of packages where possible. Remembering the whole ajv/eslint story from a few years ago is a good example of why. Another reason is to simply reduce the surface of packages that could trip vulnerability scanners. It also helps deduplicate packages which can help improve performance since the same module isn't loaded multiple times.

Now the maintenance trade-off is obviously to consider. I'm very familiar with rollup so for me it wouldn't be much of a burden. It might be different for you so I also understand if this is not something you desire for your package.

the overall size of your dependency tree would (very likely) be greater after these changes, because you are bundling dependencies that can no longer be deduplicated

I don't follow what you mean here. The only file that'd be bundled into table would be this one:

https://github.com/ajv-validator/ajv/blob/fe591439f34e24030f69df9eb8d91e6d037a3af7/lib/compile/equal.js#L1-L5

It simply aliases to fast-deep-equal. This changes pulls that alias into table and make fast-deep-equal a direct dependency. ajv itself and all its other dependencies would become dev dependency.