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

Feature request: Add size and count of unique deps to Deno info #6468

Closed
cknight opened this issue Jun 25, 2020 · 6 comments
Closed

Feature request: Add size and count of unique deps to Deno info #6468

cknight opened this issue Jun 25, 2020 · 6 comments
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)

Comments

@cknight
Copy link
Contributor

cknight commented Jun 25, 2020

Currently deno info mod.ts will produce a dependency tree of mod.ts. It would be a nice enhancement if next to each module the size in bytes could be shown in addition to the size of the sub-tree. This would help spot bloated dependencies.

Another nice to have improvement would be a count of unique dependencies.

Possible output:

local: /development/deno/deno/std/archive/tar.ts
type: TypeScript
compiled: /.cache/deno/gen/file/development/deno/deno/std/archive/tar.ts.js
map: /.cache/deno/gen/file/development/deno/deno/std/archive/tar.ts.js.map
deps: 5 unique
file:///development/deno/deno/std/archive/tar.ts (1,863 bytes, total = 32,163 bytes)
  ├─┬ file:///development/deno/deno/std/io/readers.ts (15,612 bytes, total = 22,252 bytes)
  │ └── file:///development/deno/deno/std/encoding/utf8.ts (6,810 bytes)
  ├─┬ file:///development/deno/deno/std/io/bufio.ts (5,773 bytes, total = 15,223 bytes)
  │ ├── file:///development/deno/deno/std/bytes/mod.ts (8,220 bytes)
  │ └── file:///development/deno/deno/std/_util/assert.ts (1,223 bytes)
  └── file:///development/deno/deno/std/_util/assert.ts (1,223 bytes)

(byte numbers above are fictional, don't add up and are for illustrative purposes only)

@ry
Copy link
Member

ry commented Jun 25, 2020

Sure sounds like a useful nice feature.

@ry ry added cli related to cli/ dir feat new feature (which has been agreed to/accepted) labels Jun 25, 2020
@KrisChambers
Copy link
Contributor

I wouldn't mind trying to tackle this.

@KrisChambers
Copy link
Contributor

I noticed a todo left by @bartlomieju. They mention that the print_file_info function is a repeat of the whole compilation stack.

One thing I am a bit confused by was the Deps in deno.core and ModuleGraph in deno.cli. They both seem to be generating a dependency graph, except Deps seems a bit more specialized to generating it for deno info. I was curious if it would be alright to try and turn this code into some traversals of the ModuleGraph or if I am missing something subtle here.

I think this will do a few things:

  1. Removes the need to construct a second dependency tree.
  2. Removes some code from deno.core. As far as I can tell Deps is only used for this one purpose.
  3. Addresses the todo by only using the one part of the compilation stack we really need (the dependency tree).

Any feedback / insight would be appreciated. This is my first feature.

@bartlomieju
Copy link
Member

@KrisChambers thanks for interest on working in this PR. Your diagnosis is right, however I don't have a straight answer for that. If you want to implement this feature, the best approach would be to annotate each entry in the tree with the length of the source code (that is already available on ModuleGraphFile struct), and use human_size() method available in main.rs.

As for refactoring Deps it's definitely a big refactor as it's touching both cli/ and core/ - I'd suggest not taking it up as a first issue, besides @piscisaureus is currently working on module system refactor in core/ so it might be better to try and tackle it after the refactor is complete.

@KrisChambers
Copy link
Contributor

Thanks for the reply @bartlomieju. I agree, this is probably the most straight forward way to approach it using the ModuleGraph.

I will leave Deps untouched for now and take a look at it once @piscisaureus has finished the module system refactor.

@bartlomieju
Copy link
Member

Closed in #6786

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

No branches or pull requests

4 participants