Bug 1363768 - Prevent creating constant elements during render;r=nchevobbe draft
authorBrian Grinstead <bgrinstead@mozilla.com>
Thu, 11 May 2017 09:59:49 -0700
changeset 576350 5d3dad5427ab1f72d530c8363cddb02b8bfa6d26
parent 576334 3b96f277325747fe668ca8cd896d2f581238e4ee
child 628166 ef3727cc0d940cd4690e9323d6585167c85cfd35
push id58328
push userbgrinstead@mozilla.com
push dateThu, 11 May 2017 16:59:58 +0000
reviewersnchevobbe
bugs1363768
milestone55.0a1
Bug 1363768 - Prevent creating constant elements during render;r=nchevobbe MozReview-Commit-ID: IPWczrp1TOA
devtools/client/webconsole/new-console-output/components/message-icon.js
devtools/client/webconsole/new-console-output/components/message-indent.js
devtools/client/webconsole/new-console-output/components/message-repeat.js
devtools/client/webconsole/new-console-output/components/message.js
devtools/client/webconsole/new-console-output/test/components/message-repeat.test.js
--- a/devtools/client/webconsole/new-console-output/components/message-icon.js
+++ b/devtools/client/webconsole/new-console-output/components/message-icon.js
@@ -8,25 +8,35 @@
 
 // React & Redux
 const {
   DOM: dom,
   PropTypes
 } = require("devtools/client/shared/vendor/react");
 const {l10n} = require("devtools/client/webconsole/new-console-output/utils/messages");
 
+// Store common icons so they can be used without recreating the element
+// during render.
+const CONSTANT_ICONS = {
+  "error": getIconElement("level.error"),
+  "warn": getIconElement("level.warn"),
+  "info": getIconElement("level.info"),
+  "log": getIconElement("level.log"),
+  "debug": getIconElement("level.debug"),
+};
+
+function getIconElement(level) {
+  return dom.span({
+    className: "icon",
+    title: l10n.getStr(level),
+  });
+}
+
 MessageIcon.displayName = "MessageIcon";
-
 MessageIcon.propTypes = {
   level: PropTypes.string.isRequired,
 };
-
 function MessageIcon(props) {
   const { level } = props;
-
-  const title = l10n.getStr("level." + level);
-  return dom.span({
-    className: "icon",
-    title
-  });
+  return CONSTANT_ICONS[level] || getIconElement(level);
 }
 
 module.exports = MessageIcon;
--- a/devtools/client/webconsole/new-console-output/components/message-indent.js
+++ b/devtools/client/webconsole/new-console-output/components/message-indent.js
@@ -8,20 +8,28 @@
 
 // React & Redux
 const {
   DOM: dom,
 } = require("devtools/client/shared/vendor/react");
 
 const INDENT_WIDTH = 12;
 
-function MessageIndent(props) {
-  const { indent } = props;
+// Store common indents so they can be used without recreating the element
+// during render.
+const CONSTANT_INDENTS = [getIndentElement(0), getIndentElement(1)];
+
+function getIndentElement(indent) {
   return dom.span({
     className: "indent",
     style: {"width": indent * INDENT_WIDTH}
   });
 }
 
+function MessageIndent(props) {
+  const { indent } = props;
+  return CONSTANT_INDENTS[indent] || getIndentElement(indent);
+}
+
 module.exports.MessageIndent = MessageIndent;
 
 // Exported so we can test it with unit tests.
 module.exports.INDENT_WIDTH = INDENT_WIDTH;
--- a/devtools/client/webconsole/new-console-output/components/message-repeat.js
+++ b/devtools/client/webconsole/new-console-output/components/message-repeat.js
@@ -19,20 +19,15 @@ const messageRepeatsTooltip = l10n.getSt
 MessageRepeat.displayName = "MessageRepeat";
 
 MessageRepeat.propTypes = {
   repeat: PropTypes.number.isRequired
 };
 
 function MessageRepeat(props) {
   const { repeat } = props;
-
-  if (!repeat || repeat < 2) {
-    return null;
-  }
-
   return dom.span({
     className: "message-repeats",
     title: PluralForm.get(repeat, messageRepeatsTooltip).replace("#1", repeat)
   }, repeat);
 }
 
 module.exports = MessageRepeat;
--- a/devtools/client/webconsole/new-console-output/components/message.js
+++ b/devtools/client/webconsole/new-console-output/components/message.js
@@ -186,17 +186,18 @@ const Message = createClass({
               ? serviceContainer.sourceMapService
               : undefined
           }) : null
         )));
     } else {
       notesNodes = [];
     }
 
-    const repeat = MessageRepeat({repeat: this.props.repeat});
+    const repeat = this.props.repeat && this.props.repeat > 1 ?
+      MessageRepeat({repeat: this.props.repeat}) : null;
 
     let onFrameClick;
     if (serviceContainer && frame) {
       if (source === MESSAGE_SOURCE.CSS) {
         onFrameClick = serviceContainer.onViewSourceInStyleEditor;
       } else if (/^Scratchpad\/\d+$/.test(frame.source)) {
         onFrameClick = serviceContainer.onViewSourceInScratchpad;
       } else {
--- a/devtools/client/webconsole/new-console-output/test/components/message-repeat.test.js
+++ b/devtools/client/webconsole/new-console-output/test/components/message-repeat.test.js
@@ -11,13 +11,9 @@ const {
 } = require("devtools/client/webconsole/new-console-output/test/helpers");
 
 describe("MessageRepeat component:", () => {
   it("renders repeated value correctly", () => {
     const rendered = renderComponent(MessageRepeat, { repeat: 99 });
     expect(rendered.classList.contains("message-repeats")).toBe(true);
     expect(rendered.textContent).toBe("99");
   });
-
-  it("does not render un-repeated value", () => {
-    expect(MessageRepeat({ repeat: 1 })).toBe(null);
-  });
 });