Avoid adding trailing semicolons to inline styles. (facebook#9550)

* Avoid adding trailing semicolons to inline styles. * Prettify. * For sake of performance, avoid messing around with arrays. * Change approach to avoid calling .substring.
sebmarkbage · Jun 5, 2017 · 5aa3153 · 5aa3153
1 parent bc23cc3
commit 5aa3153
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 11 deletions.
diff --git a/src/renderers/dom/shared/CSSPropertyOperations.js b/src/renderers/dom/shared/CSSPropertyOperations.js
@@ -194,6 +194,7 @@ var CSSPropertyOperations = {
    */
   createMarkupForStyles: function(styles, component) {
     var serialized = '';
+    var delimiter = '';
     for (var styleName in styles) {
       if (!styles.hasOwnProperty(styleName)) {
         continue;
@@ -203,9 +204,10 @@ var CSSPropertyOperations = {
         warnValidStyle(styleName, styleValue, component);
       }
       if (styleValue != null) {
-        serialized += processStyleName(styleName) + ':';
-        serialized +=
-          dangerousStyleValue(styleName, styleValue, component) + ';';
+        serialized += delimiter + processStyleName(styleName) + ':';
+        serialized += dangerousStyleValue(styleName, styleValue, component);
+
+        delimiter = ';';
       }
     }
     return serialized || null;

diff --git a/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
@@ -29,7 +29,7 @@ describe('CSSPropertyOperations', () => {
         backgroundColor: '#3b5998',
         display: 'none',
       }),
-    ).toBe('background-color:#3b5998;display:none;');
+    ).toBe('background-color:#3b5998;display:none');
   });
 
   it('should ignore undefined styles', () => {
@@ -38,7 +38,7 @@ describe('CSSPropertyOperations', () => {
         backgroundColor: undefined,
         display: 'none',
       }),
-    ).toBe('display:none;');
+    ).toBe('display:none');
   });
 
   it('should ignore null styles', () => {
@@ -47,7 +47,7 @@ describe('CSSPropertyOperations', () => {
         backgroundColor: null,
         display: 'none',
       }),
-    ).toBe('display:none;');
+    ).toBe('display:none');
   });
 
   it('should return null for no styles', () => {
@@ -67,7 +67,7 @@ describe('CSSPropertyOperations', () => {
         opacity: 0.5,
         padding: '4px',
       }),
-    ).toBe('left:0;margin:16px;opacity:0.5;padding:4px;');
+    ).toBe('left:0;margin:16px;opacity:0.5;padding:4px');
   });
 
   it('should trim values', () => {
@@ -77,7 +77,7 @@ describe('CSSPropertyOperations', () => {
         opacity: 0.5,
         right: ' 4 ',
       }),
-    ).toBe('left:16;opacity:0.5;right:4;');
+    ).toBe('left:16;opacity:0.5;right:4');
   });
 
   it('should not append `px` to styles that might need a number', () => {
@@ -87,7 +87,7 @@ describe('CSSPropertyOperations', () => {
       var styles = {};
       styles[property] = 1;
       expect(CSSPropertyOperations.createMarkupForStyles(styles)).toMatch(
-        /:1;$/,
+        /:1$/,
       );
     });
   });
@@ -98,7 +98,7 @@ describe('CSSPropertyOperations', () => {
         msTransition: 'none',
         MozTransition: 'none',
       }),
-    ).toBe('-ms-transition:none;-moz-transition:none;');
+    ).toBe('-ms-transition:none;-moz-transition:none');
   });
 
   it('should set style attribute when styles exist', () => {

diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
@@ -1206,7 +1206,7 @@ describe('ReactDOMComponent', () => {
           ),
         ),
       ).toBe(
-        '<div title="&#x27;&quot;&lt;&gt;&amp;" style="text-align:&#x27;&quot;&lt;&gt;&amp;;">' +
+        '<div title="&#x27;&quot;&lt;&gt;&amp;" style="text-align:&#x27;&quot;&lt;&gt;&amp;">' +
           '&#x27;&quot;&lt;&gt;&amp;' +
           '</div>',
       );