Merge pull request #109 from remarkablemark/chore/improvements

chore: skip <script> tag, improve test coverage, tidy code

Author: remarkablemark

lib/dom-to-react.js

@@ -2,6 +2,10 @@ var React = require('react');
 var attributesToProps = require('./attributes-to-props');
 var utilities = require('./utilities');
 
+var cloneElement = React.cloneElement;
+var createElement = React.createElement;
+var isValidElement = React.isValidElement;
+
 /**
  * Converts DOM nodes to React elements.
  *
@@ -14,27 +18,27 @@ function domToReact(nodes, options) {
   options = options || {};
   var result = [];
   var node;
-  var isReplacePresent = typeof options.replace === 'function';
-  var replacement;
+  var hasReplace = typeof options.replace === 'function';
+  var replaceElement;
   var props;
   var children;
 
   for (var i = 0, len = nodes.length; i < len; i++) {
     node = nodes[i];
 
-    // replace with custom React element (if applicable)
-    if (isReplacePresent) {
-      replacement = options.replace(node);
+    // replace with custom React element (if present)
+    if (hasReplace) {
+      replaceElement = options.replace(node);
 
-      if (React.isValidElement(replacement)) {
+      if (isValidElement(replaceElement)) {
         // specify a "key" prop if element has siblings
         // https://fb.me/react-warning-keys
         if (len > 1) {
-          replacement = React.cloneElement(replacement, {
-            key: replacement.key || i
+          replaceElement = cloneElement(replaceElement, {
+            key: replaceElement.key || i
           });
         }
-        result.push(replacement);
+        result.push(replaceElement);
         continue;
       }
     }
@@ -52,11 +56,11 @@ function domToReact(nodes, options) {
 
     children = null;
 
-    // node type for <script> is "script"
     // node type for <style> is "style"
-    if (node.type === 'script' || node.type === 'style') {
-      // prevent text in <script> or <style> from being escaped
-      // https://facebook.github.io/react/tips/dangerously-set-inner-html.html
+    // skip node type "script" as react-dom does not render the contents
+    if (node.type === 'style') {
+      // prevent text in <style> from being escaped
+      // https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
       if (node.children[0]) {
         props.dangerouslySetInnerHTML = {
           __html: node.children[0].data
@@ -84,7 +88,7 @@ function domToReact(nodes, options) {
       props.key = i;
     }
 
-    result.push(React.createElement(node.name, props, children));
+    result.push(createElement(node.name, props, children));
   }
 
   return result.length === 1 ? result[0] : result;

test/attributes-to-props.js

@@ -217,6 +217,20 @@ describe('attributesToProps', () => {
         }
       );
     });
+
+    [Object, Array, Number, Date].forEach(type => {
+      it(`throws an error when attributes.style=${type.name}`, () => {
+        assert.throws(
+          () => {
+            attributesToProps({ style: type });
+          },
+          {
+            name: 'TypeError',
+            message: 'First argument must be a string.'
+          }
+        );
+      });
+    });
   });
 
   describe('when utilties.PRESERVE_CUSTOM_ATTRIBUTES=false', () => {

test/dom-to-react.js

@@ -56,23 +56,9 @@ describe('dom-to-react parser', () => {
     );
   });
 
-  it('does not escape <script> content', () => {
+  it('does not parse <script> content', () => {
     const html = data.html.script;
-    const reactElement = domToReact(htmlToDOM(html));
-
-    assert(React.isValidElement(reactElement));
-    assert.deepEqual(
-      reactElement,
-      React.createElement(
-        'script',
-        {
-          dangerouslySetInnerHTML: {
-            __html: 'alert(1 < 2);'
-          }
-        },
-        null
-      )
-    );
+    assert.deepEqual(domToReact(htmlToDOM(html)), []);
   });
 
   it('does not escape <style> content', () => {

test/helpers/data.json

@@ -4,7 +4,7 @@
     "multiple": "<p>foo</p><p>bar</p>",
     "nested": "<div><p>foo <em>bar</em></p></div>",
     "attributes": "<hr id=\"foo\" class=\"bar baz\" style=\"background: #fff; text-align: center;\" data-foo=\"bar\" />",
-    "complex": "<html><head><meta charSet=\"utf-8\"/><title>Title</title><link rel=\"stylesheet\" href=\"style.css\"/></head><body><header id=\"header\">Header</header><h1 style=\"color:#000;font-size:42px\">Heading</h1><hr/><p>Paragraph</p><img src=\"image.jpg\"/><div class=\"class1 class2\">Some <em>text</em>.</div><script>alert();</script></body></html>",
+    "complex": "<html><head><meta charSet=\"utf-8\"/><title>Title</title><link rel=\"stylesheet\" href=\"style.css\"/></head><body><header id=\"header\">Header</header><h1 style=\"color:#000;font-size:42px\">Heading</h1><hr/><p>Paragraph</p><img src=\"image.jpg\"/><div class=\"class1 class2\">Some <em>text</em>.</div></body></html>",
     "textarea": "<textarea>foo</textarea>",
     "script": "<script>alert(1 < 2);</script>",
     "style": "<style>body > .foo { color: #f00; }</style>",

test/html-to-react.js

@@ -1,38 +1,38 @@
 const assert = require('assert');
 const React = require('react');
-const Parser = require('../');
+const parse = require('../');
 const { data, render } = require('./helpers/');
 
 describe('html-to-react', () => {
   describe('parser', () => {
     [undefined, null, {}, [], 42].forEach(value => {
       it(`throws an error if first argument is ${value}`, () => {
         assert.throws(() => {
-          Parser(value);
+          parse(value);
         }, TypeError);
       });
     });
 
     it('returns string if it cannot be parsed as HTML', () => {
-      assert.equal(Parser('foo'), 'foo');
+      assert.equal(parse('foo'), 'foo');
     });
 
     it('converts single HTML element to React', () => {
       const html = data.html.single;
-      const reactElement = Parser(html);
+      const reactElement = parse(html);
       assert.equal(render(reactElement), html);
     });
 
     it('converts single HTML element and ignores comment', () => {
       const html = data.html.single;
       // comment should be ignored
-      const reactElement = Parser(html + data.html.comment);
+      const reactElement = parse(html + data.html.comment);
       assert.equal(render(reactElement), html);
     });
 
     it('converts multiple HTML elements to React', () => {
       const html = data.html.multiple;
-      const reactElements = Parser(html);
+      const reactElements = parse(html);
       assert.equal(
         render(React.createElement('div', {}, reactElements)),
         '<div>' + html + '</div>'
@@ -41,32 +41,26 @@ describe('html-to-react', () => {
 
     it('converts complex HTML to React', () => {
       const html = data.html.complex;
-      const reactElement = Parser(data.html.doctype + html);
-      assert.equal(render(reactElement), html);
-    });
-
-    it('converts empty <script> to React', () => {
-      const html = '<script></script>';
-      const reactElement = Parser(html);
+      const reactElement = parse(data.html.doctype + html);
       assert.equal(render(reactElement), html);
     });
 
     it('converts empty <style> to React', () => {
       const html = '<style></style>';
-      const reactElement = Parser(html);
+      const reactElement = parse(html);
       assert.equal(render(reactElement), html);
     });
 
     it('converts SVG to React', () => {
       const svg = data.svg.complex;
-      const reactElement = Parser(svg);
+      const reactElement = parse(svg);
       assert.equal(render(reactElement), svg);
     });
 
     it('decodes HTML entities', () => {
       const encodedEntities = 'asdf &amp; &yuml; &uuml; &apos;';
       const decodedEntities = "asdf & ÿ ü '";
-      const reactElement = Parser('<i>' + encodedEntities + '</i>');
+      const reactElement = parse('<i>' + encodedEntities + '</i>');
       assert.equal(reactElement.props.children, decodedEntities);
     });
   });
@@ -75,7 +69,7 @@ describe('html-to-react', () => {
     describe('replace', () => {
       it('overrides the element if replace is valid', () => {
         const html = data.html.complex;
-        const reactElement = Parser(html, {
+        const reactElement = parse(html, {
           replace: node => {
             if (node.name === 'title') {
               return React.createElement('title', {}, 'Replaced Title');
@@ -90,7 +84,7 @@ describe('html-to-react', () => {
 
       it('does not override the element if replace is invalid', () => {
         const html = data.html.complex;
-        const reactElement = Parser(html, {
+        const reactElement = parse(html, {
           replace: node => {
             if (node.attribs && node.attribs.id === 'header') {
               return {
@@ -111,3 +105,9 @@ describe('html-to-react', () => {
     });
   });
 });
+
+describe('dom-to-react', () => {
+  it('exports domToReact', () => {
+    assert.equal(parse.domToReact, require('../lib/dom-to-react'));
+  });
+});