Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JS] Remove unnecessary production dependencies #40108

Open
jschaf opened this issue Feb 17, 2024 · 5 comments
Open

[JS] Remove unnecessary production dependencies #40108

jschaf opened this issue Feb 17, 2024 · 5 comments

Comments

@jschaf
Copy link

jschaf commented Feb 17, 2024

Describe the enhancement requested

The Apache Arrow JS bundle includes a few unnecessary packages that increase the dependency graph. The culprits are:

  • command-line-usage: used by integration.ts, json-to-arrow.ts, and arrowtocsv.ts. I think these can move to dev dependencies since it looks like the intent of the scripts is integration testing and benchmarking.
  • @swc/helpers: Introduced in GH-37657: [JS] Run bin scripts with ts-node #38500. @swc/helpers is never imported. I think it's used for
    "swc": true
    . If ts-node is only for development, this can move to devDependencies.
  • @types/node: Should move to devDependencies. I don't think there's any benefit to having it listed as a production dependency.
  • @types/command-line-args: move to devDependencies
  • @types/command-line-usage: move to devDepdencies
  • json-bignum: used by integration.ts, json-to-arrow.ts, and arrowtocsv.ts. Can move to dev dependencies.

Here's the graph of the current dependencies.

image

Component(s)

JavaScript

@domoritz
Copy link
Member

arrow2csv is a bin provided by the library so we need to keep it as a dependency. We could probably remove that script, though but that's for another day. I removed swc in #41274. The types should be in the dependencies so people get the right versions.

@jschaf
Copy link
Author

jschaf commented Apr 18, 2024

The types should be in the dependencies so people get the right versions.

I think we only need to export types transitively used by exported libraries, not exported binaries. Since @types/command-line-args, and @types/command-line-usage are only used for the arrow2csv command and integration tests, they can probably live in dev dependencies.

Similarly, I suspect @node/types is only used for binaries and integration tests, but that's harder to verify.

We could probably remove that script, though but that's for another day

Alternately, publish as a separate package. Doesn't look like moving or deleting the file would break too much (usages).

@domoritz
Copy link
Member

Good point. I'll update the pull request.

@trxcllnt
Copy link
Contributor

arrow2csv is a bin script for library consumers to use, e.g. curl /my/cool/api | npx arrow2csv | less

@domoritz
Copy link
Member

Yeah, let's not remove it for now, that's a separate issue.

One could argue that cli tools should be separate packages and that js isn't the ideal language for cli tools (which is why I made https://github.com/domoritz/arrow-tools/tree/main/crates/csv2arrow ;-)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants