feedback

GoogleChrome · May 12, 2018 · 2a2af27 · 2a2af27
1 parent 60a056f
commit 2a2af27
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 8 deletions.
diff --git a/lighthouse-core/report/html/renderer/category-renderer.js b/lighthouse-core/report/html/renderer/category-renderer.js
@@ -26,11 +26,20 @@ class CategoryRenderer {
   /**
    * @param {!ReportRenderer.AuditJSON} audit
    * @param {number} index
-   * @param {DocumentFragment=} providedTmpl
    * @return {!Element}
    */
-  renderAudit(audit, index, providedTmpl) {
-    const tmpl = providedTmpl || this.dom.cloneTemplate('#tmpl-lh-audit', this.templateContext);
+  renderAudit(audit, index) {
+    const tmpl = this.dom.cloneTemplate('#tmpl-lh-audit', this.templateContext);
+    return this.populateAuditValues(audit, index, tmpl);
+  }
+
+  /**
+   * @param {!ReportRenderer.AuditJSON} audit
+   * @param {number} index
+   * @param {!DocumentFragment} tmpl
+   * @return {!Element}
+   */
+  populateAuditValues(audit, index, tmpl) {
     const auditEl = this.dom.find('.lh-audit', tmpl);
     auditEl.id = audit.result.name;
     const displayTextEl = this.dom.find('.lh-audit__display-text', auditEl);

diff --git a/lighthouse-core/report/html/renderer/performance-category-renderer.js b/lighthouse-core/report/html/renderer/performance-category-renderer.js
@@ -46,7 +46,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
    */
   _renderOpportunity(audit, index, scale) {
     const oppTmpl = this.dom.cloneTemplate('#tmpl-lh-opportunity', this.templateContext);
-    const element = this.renderAudit(audit, index, oppTmpl);
+    const element = this.populateAuditValues(audit, index, oppTmpl);
     element.classList.add(`lh-audit--${Util.calculateRating(audit.result.score)}`);
     element.id = audit.result.name;
 

diff --git a/lighthouse-core/report/html/renderer/util.js b/lighthouse-core/report/html/renderer/util.js
@@ -39,18 +39,16 @@ class Util {
     if (typeof displayValue === 'string') return displayValue;
     if (!displayValue) return '';
 
-    // Don't mutate the value
-    displayValue = [...displayValue];
     const replacementRegex = /%([0-9]*(\.[0-9]+)?d|s)/;
-    const template = /** @type {string} */ (displayValue.shift());
+    const template = /** @type {string} */ (displayValue[0]);
     if (typeof template !== 'string') {
       // First value should always be the format string, but we don't want to fail to build
       // a report, return a placeholder.
       return 'UNKNOWN';
     }
 
     let output = template;
-    for (const replacement of displayValue) {
+    for (const replacement of displayValue.slice(1)) {
       if (!replacementRegex.test(output)) {
         // eslint-disable-next-line no-console
         console.warn('Too many replacements given');

diff --git a/lighthouse-core/test/report/html/renderer/util-test.js b/lighthouse-core/test/report/html/renderer/util-test.js
@@ -135,4 +135,11 @@ describe('util helpers', () => {
       'Too many replacements given',
     ]);
   });
+
+  it('does not mutate the provided array', () => {
+    const displayValue = ['one:%s, two:%s', 'foo', 'bar'];
+    const cloned = JSON.parse(JSON.stringify(displayValue));
+    Util.formatDisplayValue(displayValue);
+    assert.deepStrictEqual(displayValue, cloned, 'displayValue was mutated');
+  });
 });