fix(common): handle JS floating point errors in percent pipe

Fixes angular#20136
ocombe · Dec 20, 2017 · deadc4a · deadc4a
1 parent 3846f19
commit deadc4a
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 9 deletions.
diff --git a/packages/common/src/i18n/format_number.ts b/packages/common/src/i18n/format_number.ts
@@ -46,19 +46,14 @@ export function formatNumber(
     num = value;
   }
 
-  if (style === NumberFormatStyle.Percent) {
-    num = num * 100;
-  }
-
-  const numStr = Math.abs(num) + '';
   const pattern = parseNumberFormat(format, getLocaleNumberSymbol(locale, NumberSymbol.MinusSign));
   let formattedText = '';
   let isZero = false;
 
   if (!isFinite(num)) {
     formattedText = getLocaleNumberSymbol(locale, NumberSymbol.Infinity);
   } else {
-    const parsedNumber = parseNumber(numStr);
+    const parsedNumber = parseNumber(num, style);
 
     let minInt = pattern.minInt;
     let minFraction = pattern.minFrac;
@@ -250,10 +245,11 @@ interface ParsedNumber {
 }
 
 /**
- * Parse a number (as a string)
+ * Parse a number
  * Significant bits of this parse algorithm came from https://github.com/MikeMcl/big.js/
  */
-function parseNumber(numStr: string): ParsedNumber {
+function parseNumber(num: number, style: NumberFormatStyle): ParsedNumber {
+  let numStr = Math.abs(num) + '';
   let exponent = 0, digits, integerLen;
   let i, j, zeros;
 
@@ -273,6 +269,18 @@ function parseNumber(numStr: string): ParsedNumber {
     integerLen = numStr.length;
   }
 
+  // for the percent pipe, we multiply the number by 100
+  if (style === NumberFormatStyle.Percent) {
+    // getting the current number of decimals
+    const fractionLen = numStr.length - integerLen;
+    if (fractionLen === 0) {
+      numStr += '00';
+    } else if (fractionLen === 1) {
+      numStr += '0';
+    }
+    integerLen += 2;
+  }
+
   // Count the number of leading zeros.
   for (i = 0; numStr.charAt(i) === ZERO_CHAR; i++) { /* empty */
   }
@@ -356,11 +364,20 @@ function roundNumber(parsedNumber: ParsedNumber, minFrac: number, maxFrac: numbe
   // Pad out with zeros to get the required fraction length
   for (; fractionLen < Math.max(0, fractionSize); fractionLen++) digits.push(0);
 
-
+  let fixTrailingZeros = fractionSize !== 0;
+  const minLen = minFrac + parsedNumber.integerLen;
   // Do any carrying, e.g. a digit was rounded up to 10
   const carry = digits.reduceRight(function(carry, d, i, digits) {
     d = d + carry;
     digits[i] = d % 10;
+    if (fixTrailingZeros) {
+      // do not keep meaningless fractional trailing zeros (e.g. 15.52000 --> 15.52)
+      if (digits[i] === 0 && i >= minLen) {
+        digits.pop();
+      } else {
+        fixTrailingZeros = false;
+      }
+    }
     return Math.floor(d / 10);
   }, 0);
   if (carry) {

diff --git a/packages/common/test/pipes/number_pipe_spec.ts b/packages/common/test/pipes/number_pipe_spec.ts
@@ -79,6 +79,21 @@ export function main() {
           expect(pipe.transform(1.2, '.2')).toEqual('120.00%');
           expect(pipe.transform(1.2, '4.2')).toEqual('0,120.00%');
           expect(pipe.transform(1.2, '4.2', 'fr')).toEqual('0 120,00 %');
+          // see issue #20136
+          expect(pipe.transform(0.12345674, '0.0-10')).toEqual('12.345674%');
+          expect(pipe.transform(0, '0.0-10')).toEqual('0%');
+          expect(pipe.transform(1, '0.0-10')).toEqual('100%');
+          expect(pipe.transform(0.1, '0.0-10')).toEqual('10%');
+          expect(pipe.transform(0.12, '0.0-10')).toEqual('12%');
+          expect(pipe.transform(0.123, '0.0-10')).toEqual('12.3%');
+          expect(pipe.transform(12.3456, '0.0-10')).toEqual('1,234.56%');
+          expect(pipe.transform(12.345600, '0.0-10')).toEqual('1,234.56%');
+          expect(pipe.transform(12.345699999, '0.0-6')).toEqual('1,234.57%');
+          expect(pipe.transform(12.345699999, '0.4-6')).toEqual('1,234.5700%');
+          expect(pipe.transform(100, '0.4-6')).toEqual('10,000.0000%');
+          expect(pipe.transform(100, '0.0-10')).toEqual('10,000%');
+          expect(pipe.transform(1.5e2)).toEqual('15,000%');
+          expect(pipe.transform(1e100)).toEqual('1E+102%');
         });
 
         it('should not support other objects', () => {