maximumFractionDigits not working as expected

[doomsower]

This code

console.log({
    size,
    def: prettyBytes(size),
    max: prettyBytes(size, { maximumFractionDigits: 1 }),
    minMax: prettyBytes(size, {
      maximumFractionDigits: 1,
      minimumFractionDigits: 1,
    }),
  });

Results in following output:

{"def": "60 MB", "max": "59.952784 MB", "minMax": "59.952784 MB", "size": 59952784}

I expect that both max and minMax should be 59.9 MB.

I'm using pretty-bytes@^5.6.0

[sindresorhus]

// @nmoinvaz

[nmoinvaz]

@sindresorhus I am not doing well and will not be able to attend to this for some time.

[sindresorhus]

@nmoinvaz No worries at all.

[sindresorhus]

I have added some failing tests for this: 853dc19

[mianlang]

As I tested in my
env 1: pretty-bytes@5.6.0, Windows 11 21H2, node v14.17.6 and
env 2: pretty-bytes@5.6.0, Ubuntu 20.04.3 LTS, node v17.0.1,

I got same result:

{ size: 59952784, def: '60 MB', max: '60 MB', minMax: '60.0 MB' }

There seems to be no fraction digit problems on my devices.

I assume the rounding 59952784 -> 60.0 is reasonable. And the failing tests 853dc19 with -> 59.9 might need some modifications.
If so, maybe something like 3945, 2 digits -> 3.95 (which my snippet output) could also be added to the tests (or maybe and the readme) to clarify the rounding.

[nmoinvaz]

minimumFractionDigits and maximumFractionDigits functionality is provided by JavaScript's Number.toLocaleString function options parameter (described here in Intl.NumberFormat).

Intl.NumberFormat appears to always perform rounding even though some documentation on the web does not mention it.

[nmoinvaz]

There is an ECMA proposal to support additional rounding options for Intl.NumberFormat options.
https://github.com/tc39/proposal-intl-numberformat-v3

[ghost]

Hey @sindresorhus,

I've checked out the issue reported by @doomsower regarding the prettyBytes function in the pretty-bytes package, and I've got some thoughts on it.

Issue Summary:
It looks like there are a couple of issues with prettyBytes. Firstly, there are some rounding discrepancies, especially noticeable when maximumFractionDigits is set to 1. Also, there's a problem with automatic trimming of leading zeros.

Analysis:
After digging into it, two main issues became clear:

• The rounding behavior, especially with maximumFractionDigits, isn't quite what we'd expect.
• The automatic trimming of leading zeros is causing confusion and messing with the formatting consistency.

Proposed Solution:
I think we should tweak the prettyBytes function to handle formatting manually using Number.toFixed(). This way, we can ensure consistent rounding, plus avoid the automatic trimming of leading zeros.

// Format digits after the decimal point
const numString = number.toFixed(options.maximumFractionDigits);

Given the current limitations and performance issues of Intl.NumberFormat(), I suggest we steer clear of it for now (I know, it sounds a bit extreme, but it's causing more trouble than it's worth). Instead, let's provide an option to format the output as 59,9 instead of 59.9 when needed.

Conclusion:
These changes will iron out the kinks in prettyBytes and make it more accurate and consistent, providing a better user experience overall.

Looking forward to your thoughts on this approach.

Cheers,
Edu

[nmoinvaz]

https://github.com/tc39/proposal-intl-numberformat-v3 states it was shipped in Chrome 106. I think it would be better just to add support for the rounding options roundingPriority and roundingMode.

[ghost]

@nmoinvaz sollution rocks!

With the options suggested by him, when using minimumFractionDigits and maximumFractionDigits set to 1, the values now became consistent, look:

// Tested on Node.js v21.6.1

Number(60.0000).toLocaleString('fr',{minimumFractionDigits: 0, maximumFractionDigits: 0, roundingMode: 'floor'})
// Returns: '60'

Number(60.0000).toLocaleString('fr',{minimumFractionDigits: 1, maximumFractionDigits: 1, roundingMode: 'floor'})
// Returns: '60,0'

Number(59.952784).toLocaleString('fr',{minimumFractionDigits: 1, maximumFractionDigits: 1, roundingMode: 'floor'})
// Returns: '59,9'

Number(59.952784).toLocaleString('fr',{minimumFractionDigits: 2, maximumFractionDigits: 2, roundingMode: 'floor'})
// Returns: '59,95'

Following the NumberFormat docs, the only change needed to fix the problems mentioned would be to add roundingMode: 'floor' to the default locale options. Quite a simple solution but works like charm 👏