Skip to content

feat(build): Use kB in build reporter#10982

Merged
patak-cat merged 1 commit intovitejs:mainfrom
ArnaudBarre:build-kbs
Nov 22, 2022
Merged

feat(build): Use kB in build reporter#10982
patak-cat merged 1 commit intovitejs:mainfrom
ArnaudBarre:build-kbs

Conversation

@ArnaudBarre
Copy link
Copy Markdown
Member

Related to #10895

cc @Shinigami92

@bluwy
Copy link
Copy Markdown
Member

bluwy commented Nov 18, 2022

Wouldn't dividing by 1024 and showing as KiB be more accurate? Dividing by 1000 feels like it decreases the accuracy

@patak-cat
Copy link
Copy Markdown
Member

I think we should move to kB (dividing by 1000) too.

The last PR that changed the unit was:

But Chrome dev tools are using kB across the board: https://developer.chrome.com/blog/new-in-devtools-88/#consistent-kb

DevTools now consistently use kB for displaying file/memory sizes. Previously DevTools mixed kB (1000 bytes) and KiB (1024 bytes). For example, the Network panel previously used "kB" labels but actually performed calculations using KiB, which caused needless confusion.

@benmccann
Copy link
Copy Markdown
Collaborator

For kB, it seems that Ubuntu uses base 10 in all GUI applications (newer) and base 2 in all CLI applications like ls (older).

I feel like it sucks that Chrome and ls don't agree, so there won't be agreement regardless of what we pick.

But if I guess if base 10 is the trend these days we might as well go with it 🤷

@patak-cat
Copy link
Copy Markdown
Member

That is a good point with ls @benmccann. I still think that it would probably be more surprising that the user doesn't see the same numbers in the chrome network tab compared to what Vite reports, no? Hopefully, every tool ends up moving at one point.

@benmccann
Copy link
Copy Markdown
Collaborator

Yeah, I'm still slightly in favor of this PR and hope one day ls will end up moving too

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

Successfully merging this pull request may close these issues.

4 participants