bug: engineeringNotation formatter is not locale-aware

High level: Certain inputs to the engineeringNotation formatter result in unexpected and incorrect output values, in some locales.

Note: This has not manifested in usages of GitLab UI by GitLab (hence ~P4 ~S4) but the potential is there. Rather than leaving the trap in place for future users, I propose fixing it now.

More details

Special support for string inputs with thousands-separators like "1,000.00" was added in this MR: !1222 (merged). The way it is implemented assumes that the inputs are of the en-US locale or something similar. If a string from a locale that handles thousands-separators differently is passed in, unexpected behaviour occurs.

In the worst case, the number is divided by a thousand and returned as a valid result.

To Reproduce

  1. Take a number over 1000, e.g. 1000.11.
  2. Pick a locale that uses something other than a comma for digit-grouping, such as de-DE
  3. Represent this number as a locale-aware string from the locale above.
    • e.g. run (1000.11).toLocaleString('de-DE')
    • result: `"1.000,11"
  4. Call engineeringNotation with that string, e.g. `engineeringNotation('1.000,11')

Expected Behaviour

The returned result is 1k.

Actual Behaviour

The returned result is 1.

What's going on here?

The formatter assumes that comma characters are thousands-separators and that full-stop characters are decimal points. Thus it converts "1.000,11" to "1.00011" and then parses that result as a number.

What's the solution?

We don't seem to currently call the formatter with string args anyway, so we can drop the special handling of this case. If we want to model this formatter an existing library, let's go for something browser native like Intl.NumberFormat.prototype.format.

Edited by Tristan Read