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] Make the node radius respect the package size #132

Closed
styfle opened this issue Sep 26, 2022 · 13 comments
Closed

[Feature request] Make the node radius respect the package size #132

styfle opened this issue Sep 26, 2022 · 13 comments

Comments

@styfle
Copy link

styfle commented Sep 26, 2022

It would be great if there was a way to view package size visually.

This could be achieved by reading the unpackedSize prop from the registry and then rendering the node radius larger or smaller in the graph.

@broofa
Copy link
Collaborator

broofa commented Sep 26, 2022

I've thought about this previously when I first noticed the unpackedSize property in the registry data. 'Got pretty excited about it. But ultimately decided it was counter-productive. The problem is it doesn't correlate well to anything users actually care about. When users talk about "size", they're most often referring to the impact a dependency has on their app bundle. unpackedSize is often misleading in that regard. Some modules don't specify their 'files' properly, so there's lots of unnecessary cruft in the module. Other modules (like uuid) ship with multiple copies of the code to support different module formats. And modules may or may not be designed for tree-shaking, which can have a significant impact on bundle size.

For exampleuuid's unpackedSize is 123KB, but for most users it adds < 1KB to their bundle size.

tl;dr: I'm open to the idea, but I think we need a better "size" metric.

@styfle
Copy link
Author

styfle commented Sep 27, 2022

The “app bundle” size is only relevant for fronted packages.

The Install Size is still relevant and you can see there are quite a few people using Package Phobia to measure Install Size and Publish Size. https://github.com/styfle/packagephobia

The graph view would be a nice addition to narrowing down, where in the tree the bloat is coming from.

That said, a toggle between bundle size and package size would be great too!

@broofa
Copy link
Collaborator

broofa commented Oct 30, 2022

What's the difference between "Install Size and Publish Size"?

Also, when I go to https://packagephobia.com/result?p=uuid, it says it's 120KB. That is (conveniently) also the same as npm's unpackedSize but... where does that number come from?

  • The tgz bundle that NPM downloads when you do npm install uuid is 23KB
  • ... which, unpacked, installs 360KB of files on disk
  • ... which is mostly due to there being 4 different copies of the source in various module formats (60-90kb each)
  • If you do import {v4 as uuid} from 'uuid', it adds ~1KB in actual code (tree-shaken)

Conspicuously missing from this list is anything that adds up to 120KB.


The other thing I struggle with here - and this is really the bigger problem - is that no matter how we define "size", we're not really addressing the main question users have: "How does [dependency X] affect the size of my project?"

The actual impact a dependency has on a project depends on how it interacts with the project's other dependencies. For example...

graph TD

m1["m1 (1kb)"]
m2["m2 (2kb)"]
m3["m3 (4kb)"]
m4["m4 (8kb)"]
m5["m5 (16kb)"]

a[My Project] -->|dep1| m1
a -->|dep2| m2
a -->|dep3| m3

m1 --> m4
m1 --> m5
m2 --> m5
m2 --> m3

style m1 fill:#f99
style m2 fill:#9f9
style m3 fill:#99f
style m4 fill:#f99
style m5 fill:#ff9
Loading

Ignoring what the "kb" size here is measuring (file system footprint, size of imported files, tree-shaken code size... whatever), there is no definition of "size" that will properly account for the following:

  • Removing the dependency on m1 (dep1) will remove m1 and m4, but not m5
  • Removing the dependency on m2 (dep2) will only remove m2, and not m5 or m3
  • Removing the dependency on m3 (dep3) won't remove any modules at all

This sort of "massaging" of dependencies is, I think, what's really of most interest. I'd love to have a way for npmgraph to help users understand that removing a module with "900Kb of dependencies" doesn't do much if there are other dependencies in their project that rely on that same 900Kb of code.

@styfle
Copy link
Author

styfle commented Oct 30, 2022

What's the difference between "Install Size and Publish Size"?

It explains in the footer

image

... which, unpacked, installs 360KB of files on disk

The disk size can vary depending on how your block size.

See Why is the size different than size on disk? in the readme

"How does [dependency X] affect the size of my project?"

True for install size but publish size (aka unpacked size) is predictable and that is what I'm suggesting adding to npmgraph.

@fabiospampinato
Copy link

IMO this should be integrated with BundlePhobia (https://bundlephobia.com/), which measures the size of the dependency once bundled. The package size or the install size at the end of the day are just noise and irrelevant.

@styfle
Copy link
Author

styfle commented Feb 14, 2023

Bundle size depends on your choice of bundler and is not relevant for dependencies that never ship to the frontend (and are therefore never bundled).

@fabiospampinato
Copy link

@styfle it depends on the blunder but if some bundler gives you 50% larger bundles on average or something it's probably broken.

Bundle size is potentially also not irrelevant for server-side things, for example if your cli app load 2mb of code it isn't going to start quickly. It's less important though, sure.

@npmgraph npmgraph deleted a comment from fabiospampinato Feb 15, 2023
@npmgraph npmgraph deleted a comment from fabiospampinato Feb 15, 2023
@styfle
Copy link
Author

styfle commented Apr 15, 2024

How about adding an optional toggle in the bottom left corner so that only users who care about size will see it?

image

styfle added a commit to styfle/packagephobia that referenced this issue Apr 15, 2024
This project is actively maintained so more likely to get a size feature
to land.

### Related
- anvaka/npmgraph.an#27
- npmgraph/npmgraph#132
@broofa
Copy link
Collaborator

broofa commented Apr 15, 2024

Drive-by update...

  • BundlePhobia API has been a bit unreliable over the years, both in terms of QoS and the data it provides. I wouldn't mind swapping that out for something else.
  • I still think the ambiguity around "size" risks being confusing.
  • As @styfle notes, bundle size depends to some extent on the bundler, which probably contributes to the confusion.
  • Showing file-system size would've been helpful in diagnosing/reporting an issue I ran into with another project recently I had recently.

On the topic of UI for this ...

I had actually intended to say this was going to be tricky, but it looks like varying the fontsize attribute in the GraphViz markup works pretty well. For example, generating random font sizes for the nodes in the express graph looksl ike this:

CleanShot 2024-04-15 at 09 29 11

... which is actually pretty compelling (I think). The one downside to this is that it doesn't communicate any information about the absolute size of things. Does this represent a 100KB install or a 100MB install? It's unclear. There's the existing TreeMap display used for BundlePhobia data in the Module Pane, though, which does convey that information. So... maybe some combination of the two.

The upshot of this is that if I'd be open to a PR for this. Otherwise I'll try get around to this in my spare (ha!) time.

@styfle
Copy link
Author

styfle commented Apr 15, 2024

I think the font size is a good idea 👍

If the value in the right side panel is changed from "Bundle Size" to "Unpacked Size" and uses the value from the registry, that should help make it more clear.

You can see "Unpacked Size" is already displayed on npmjs.org as seen here:

image

@styfle
Copy link
Author

styfle commented Apr 15, 2024

Oh I just realized that the "Bundle Size" actually does work for some existing packages so probably keep that but just add the "Unpacked Size"

@broofa
Copy link
Collaborator

broofa commented Apr 17, 2024

Put up #211 to try out the fontsize-scaling idea using actual unpackedSize data. We can continue conversation over there...

(@styfle I see you've already found that PR. I updated the description there with some comments.)

@broofa broofa closed this as completed in aa33b2c Apr 18, 2024
@broofa
Copy link
Collaborator

broofa commented Apr 18, 2024

BTW, this feature is controlled by the sizing hash param, if anyone is building URLs. Not yet documented in the README, or supported in npmgraph-cli, though. Work for another day. :-)

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

No branches or pull requests

3 participants