Bug 1406311 - string-format: stop using spread arguments in l10n getFormatStr;r=bgrins draft
authorJulian Descottes <jdescottes@mozilla.com>
Thu, 12 Oct 2017 15:07:27 +0200
changeset 680935 db2e708e719b04354afb1142f506abc3fde9b07d
parent 680934 19db44fe15d2deb3723b4ed0dd5e47b161cee8a9
child 736018 5bf72e72e6f597075195601e900033035ad521c5
push id84682
push userjdescottes@mozilla.com
push dateMon, 16 Oct 2017 16:12:33 +0000
reviewersbgrins
bugs1406311
milestone58.0a1
Bug 1406311 - string-format: stop using spread arguments in l10n getFormatStr;r=bgrins This does provide a decent improvement (~10% for getFormatStr in some of my tests). MozReview-Commit-ID: 4cOKmECnaBF
devtools/shared/l10n.js
devtools/shared/string-format.js
devtools/shared/tests/unit/test_string-format.js
--- a/devtools/shared/l10n.js
+++ b/devtools/shared/l10n.js
@@ -1,15 +1,15 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 const parsePropertiesFile = require("./node-properties/node-properties");
-const { stringFormat } = require("./string-format");
+const { stringFormatNospread } = require("./string-format");
 
 const propertiesMap = {};
 
 // We need some special treatment here for webpack.
 //
 // Webpack doesn't always handle dynamic requires in the best way.  In
 // particular if it sees an unrestricted dynamic require, it will try
 // to put all the files it can find into the generated pack.  (It can
@@ -113,18 +113,18 @@ LocalizationHelper.prototype = {
 
   /**
    * L10N shortcut function.
    *
    * @param string name
    * @param array args
    * @return string
    */
-  getFormatStr: function (name, ...args) {
-    return stringFormat(this.getStr(name), ...args);
+  getFormatStr: function (name, /* ...args */) {
+    return stringFormatNospread(this.getStr(name), arguments);
   },
 
   /**
    * L10N shortcut function for numeric arguments that need to be formatted.
    * All numeric arguments will be fixed to 2 decimals and given a localized
    * decimal separator. Other arguments will be left alone.
    *
    * @param string name
--- a/devtools/shared/string-format.js
+++ b/devtools/shared/string-format.js
@@ -46,35 +46,38 @@ const re = {
   keyAccess: /^\.([a-z_][a-z_\d]*)/i,
   indexAccess: /^\[(\d+)\]/,
   sign: /^[\+\-]/
 };
 
 const SIMPLE_STRING_PATTERN = "SIMPLE_STRING_PATTERN";
 
 const cache = {};
-function stringFormat() {
-  let key = arguments[0];
-  if (!(cache[key] && cache.hasOwnProperty(key))) {
-    cache[key] = stringFormat.parse(key);
+function stringFormat(pattern) {
+  return stringFormatNospread(pattern, arguments);
+}
+
+function stringFormatNospread(pattern, args) {
+  if (!(cache[pattern] && cache.hasOwnProperty(pattern))) {
+    cache[pattern] = stringFormat.parse(pattern);
   }
-  return stringFormat.format.call(null, cache[key], arguments);
+  return stringFormat.format.call(null, cache[pattern], pattern, args);
 }
 
 // eslint-disable-next-line complexity
-stringFormat.format = function (parseTree, argv) {
+stringFormat.format = function (parseTree, pattern, argv) {
   // For simple string patterns, simply use String.prototype.replace.
   if (parseTree === SIMPLE_STRING_PATTERN) {
     let arg = argv[1];
     if (typeof arg == "function") {
       arg = arg();
     } else if (typeof arg != "string") {
       arg = String(arg);
     }
-    return argv[0].replace("%S", arg);
+    return pattern.replace("%S", arg);
   }
 
   let cursor = 1, treeLength = parseTree.length, nodeType = "", arg, output = [], i, k,
     match;
   for (i = 0; i < treeLength; i++) {
     nodeType = typeof parseTree[i];
     // parseTree item is either a string or an array (return of match() call).
     if (nodeType === "string") {
@@ -290,8 +293,9 @@ var preformattedPadding = {
 function strRepeat(input, multiplier) {
   if (multiplier >= 0 && multiplier <= 7 && preformattedPadding[input]) {
     return preformattedPadding[input][multiplier];
   }
   return Array(multiplier + 1).join(input);
 }
 
 exports.stringFormat = stringFormat;
+exports.stringFormatNospread = stringFormatNospread;
--- a/devtools/shared/tests/unit/test_string-format.js
+++ b/devtools/shared/tests/unit/test_string-format.js
@@ -5,20 +5,24 @@
 "use strict";
 
 /**
  * This unit test checks that our string formatter works with different patterns and
  * arguments.
  * Initially copied from unit tests at https://github.com/alexei/sprintf.js
  */
 
-const {stringFormat} = require("devtools/shared/string-format");
+let {
+  stringFormat: originalStringFormat,
+  stringFormatNospread,
+} = require("devtools/shared/string-format");
+
 const PI = 3.141592653589793;
 
-function run_test() {
+function testStringFormat(stringFormat) {
   // Simple patterns
   equal("%", stringFormat("%%"));
   equal("10", stringFormat("%b", 2));
   equal("A", stringFormat("%c", 65));
   equal("2", stringFormat("%d", 2));
   equal("2", stringFormat("%i", 2));
   equal("2", stringFormat("%d", "2"));
   equal("2", stringFormat("%i", "2"));
@@ -106,8 +110,17 @@ function run_test() {
   equal("2.3", stringFormat("%.1f", 2.345));
   equal("xxxxx", stringFormat("%5.5s", "xxxxxx"));
   equal("    x", stringFormat("%5.1s", "xxxxxx"));
 
   equal("foobar", stringFormat("%s", function () {
     return "foobar";
   }));
 }
+
+function run_test() {
+  testStringFormat(originalStringFormat);
+  testStringFormat(function (pattern) {
+    // Small adapter so that stringFormatNospread can be called with the same arguments
+    // as the default stringFormat.
+    return stringFormatNospread(pattern, arguments);
+  });
+}