search

d3 / d3-dsv

A parser and formatter for delimiter-separated values, such as CSV and TSV.
https://d3js.org/d3-dsv
ISC License
437 stars 76 forks source link

[RFC] Split out the CLI to a separate package #71

Closed stof closed 3 years ago

stof commented 4 years ago

Currently, this d3-dsv package depends on commander, rw and iconv-lite. But these dependencies are actually only used by the CLI tools being exposed, not when using the library directly (as done in several chart libraries). I think it might make sense to split the CLI tools into a separate package (depending on d3-dsv), so that projects only needing the library don't need these additional dependencies.

Fil commented 4 years ago

That design decision helps make sure the CLI is consistent. Same approach in d3-geo-projection etc.

I understand the desire for a smaller footprint, but this is probably not going to change. Please feel free to add arguments and we'll reopen the issue as necessary.

stof commented 4 years ago

Well, if the main reason is to ease the maintenance, a solution could be to keep developing them in the same repo, but publish them as separate packages. There are several tools helping manage mono-repositories (yarn workspaces, lerna, etc...). Would this be an acceptable approach ?

Looking at https://packagephobia.com/result?p=d3-dsv@1.2.0, it shows that the total install size is 507KB while the d3-dsv package is only 50KB. And all dependencies are about the CLI in this case. This means that the cost of this CLI for the install size is at least 450KB (at least being because part of the 50KB of d3-dsv are also about the CLI, but I haven't compared the sizes here).

mbostock commented 3 years ago

If the only goal is to decrease the published package size, I’m not motivated to adopt a monorepo, which adds complexity and development overhead.

I’m more inclined to reduce the size of the dependencies (but also this isn’t very high on my priority list). I suspect that rw could be made smaller by adding an explicit files entry to the package.json; perhaps iconv-lite could be removed by switching to Node’s TextEncoder (and presumably introducing a minimum Node version requirement); and perhaps there’s a more efficient substitute for commander (though that’s likely difficult to replace without changing behavior).