Merge pull request #16 from remarkablemark/bug-void-element

remarkablemark · web-flow · commit 20177ee68eb0 · 2016-08-29T15:00:54.000-04:00
Fix void element bug and add `key` parameter to `replace` method
diff --git a/README.md b/README.md
@@ -68,11 +68,36 @@ ReactDOM.render(
 
 ### Options
 
-#### replace(domNode)
+#### replace(domNode, key)
 
-`replace` allows you to swap an element with your own React element.
+The `replace` method allows you to swap an element with your own React element.
 
-The `domNode` object has the same schema as the output from [htmlparser2.parseDOM](https://github.com/fb55/domhandler#example).
+The method has 2 parameters:
+1. `domNode`: An object which shares the same schema as the output from [htmlparser2.parseDOM](https://github.com/fb55/domhandler#example).
+2. `key`: A number to keep track of the element. You should set it as the ["key" prop](https://fb.me/react-warning-keys) in case your element has siblings.
+
+```js
+Parser('<p id="replace">text</p>', {
+    replace: function(domNode, key) {
+        console.log(domNode);
+        // {  type: 'tag',
+        //    name: 'p',
+        //    attribs: { id: 'replace' },
+        //    children: [],
+        //    next: null,
+        //    prev: null,
+        //    parent: null }
+
+        console.log(key); // 0
+
+        return;
+        // element is not replaced because
+        // a valid React element is not returned
+    }
+});
+```
+
+Working example:
 
 ```js
 var Parser = require('html-react-parser');
@@ -81,18 +106,14 @@ var React = require('react');
 var html = '<div><p id="main">replace me</p></div>';
 
 var reactElement = Parser(html, {
-    replace: function(domNode) {
-        // example `domNode`:
-        // {  type: 'tag',
-        //    name: 'p',
-        //    attribs: { id: 'main' },
-        //    children: [],
-        //    next: null,
-        //    prev: null,
-        //    parent: [Circular] }
+    replace: function(domNode, key) {
         if (domNode.attribs && domNode.attribs.id === 'main') {
-            // element is replaced only if a valid React element is returned
-            return React.createElement('span', { style: { fontSize: '42px' } }, 'replaced!');
+            return React.createElement('span', {
+                key: key,
+                style: { fontSize: '42px' } },
+            'replaced!');
+            // equivalent jsx syntax:
+            // return <span key={key} style={{ fontSize: '42px' }}>replaced!</span>;
         }
     }
 });
diff --git a/lib/dom-to-react.js b/lib/dom-to-react.js
@@ -28,7 +28,7 @@ function domToReact(nodes, options) {
 
         // replace with custom React element (if applicable)
         if (isReplacePresent) {
-            replacement = options.replace(node);
+            replacement = options.replace(node, i); // i = key
 
             if (React.isValidElement(replacement)) {
                 result.push(replacement);
@@ -59,8 +59,8 @@ function domToReact(nodes, options) {
             if (node.name === 'textarea' && node.children[0]) {
                 props.defaultValue = node.children[0].data;
 
-            } else if (node.children) {
-                // continue recursion of creating React elements
+            // continue recursion of creating React elements (if applicable)
+            } else if (node.children && node.children.length) {
                 children = domToReact(node.children, options);
             }
 
@@ -69,7 +69,7 @@ function domToReact(nodes, options) {
             continue;
         }
 
-        // specify a `key` prop if returning an array
+        // specify a "key" prop if element has siblings
         // https://fb.me/react-warning-keys
         if (len > 1) {
             props.key = i;
diff --git a/test/data.json b/test/data.json
@@ -4,9 +4,11 @@
     "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><title>Title</title></head><body><header id=\"header\">Header</header><h1 style=\"color:#000;font-size:42px;\">Heading</h1><p>Paragraph</p><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><script>alert();</script></body></html>",
     "textarea": "<textarea>foo</textarea>",
     "script": "<script>alert(1 < 2);</script>",
+    "img": "<img src=\"http://stat.ic/img.jpg\" alt=\"Image\"/>",
+    "void": "<link/><meta/><img/><br/><hr/><input/>",
     "comment": "<!-- comment -->"
   },
   "svg": {
diff --git a/test/dom-to-react.js b/test/dom-to-react.js
@@ -7,6 +7,7 @@ var assert = require('assert');
 var React = require('react');
 var htmlToDOMServer = require('../lib/html-to-dom-server');
 var domToReact = require('../lib/dom-to-react');
+var helpers = require('./helpers/');
 var data = require('./data');
 
 /**
@@ -67,6 +68,20 @@ describe('dom-to-react parser', function() {
         );
     });
 
+    it('does not have `children` for void elements', function() {
+        var html = data.html.img;
+        var reactElement = domToReact(htmlToDOMServer(html));
+        assert(!reactElement.props.children);
+    });
+
+    it('does not throw an error for void elements', function() {
+        var html = data.html.void;
+        var reactElements = domToReact(htmlToDOMServer(html));
+        assert.doesNotThrow(function() {
+            helpers.render(React.createElement('div', {}, reactElements));
+        });
+    });
+
     it('skips HTML comments', function() {
         var html = [data.html.single, data.html.comment, data.html.single].join('');
         var reactElements = domToReact(htmlToDOMServer(html));
diff --git a/test/helpers/index.js b/test/helpers/index.js
@@ -5,6 +5,21 @@
  */
 var assert = require('assert');
 var util = require('util');
+var React = require('react');
+var ReactDOMServer = require('react-dom/server');
+
+/**
+ * Render a React element to static HTML markup.
+ *
+ * @param  {ReactElement} reactElement - The React element.
+ * @return {String}                    - The static HTML markup.
+ */
+function render(reactElement) {
+    if (!React.isValidElement(reactElement)) {
+        throw new Error(reactElement, 'is not a valid React element.');
+    }
+    return ReactDOMServer.renderToStaticMarkup(reactElement);
+}
 
 /**
  * Test for deep equality between objects that have circular references.
@@ -65,5 +80,6 @@ function deepEqualCircular(actual, expected) {
  * Export assert helpers.
  */
 module.exports = {
-    deepEqualCircular: deepEqualCircular
+    deepEqualCircular: deepEqualCircular,
+    render: render
 };
diff --git a/test/html-to-react.js b/test/html-to-react.js
@@ -5,20 +5,10 @@
  */
 var assert = require('assert');
 var React = require('react');
-var ReactDOMServer = require('react-dom/server');
 var Parser = require('../');
+var helpers = require('./helpers/');
 var data = require('./data');
 
-/**
- * Render a React element to static HTML markup.
- *
- * @param  {ReactElement} reactElement - The React element.
- * @return {String}                    - The static HTML markup.
- */
-function render(reactElement) {
-    return ReactDOMServer.renderToStaticMarkup(reactElement);
-}
-
 /**
  * Tests for `htmlToReact`.
  */
@@ -44,35 +34,35 @@ describe('html-to-react', function() {
         it('converts single HTML element to React', function() {
             var html = data.html.single;
             var reactElement = Parser(html);
-            assert.equal(render(reactElement), html);
+            assert.equal(helpers.render(reactElement), html);
         });
 
         it('converts single HTML element and ignores comment', function() {
             var html = data.html.single;
             // comment should be ignored
             var reactElement = Parser(html + data.html.comment);
-            assert.equal(render(reactElement), html);
+            assert.equal(helpers.render(reactElement), html);
         });
 
         it('converts multiple HTML elements to React', function() {
             var html = data.html.multiple;
             var reactElements = Parser(html);
             assert.equal(
-                render(React.createElement('div', {}, reactElements)),
+                helpers.render(React.createElement('div', {}, reactElements)),
                 '<div>' + html + '</div>'
             );
         });
 
         it('converts complex HTML to React', function() {
             var html = data.html.complex;
             var reactElement = Parser(html);
-            assert.equal(render(reactElement), html);
+            assert.equal(helpers.render(reactElement), html);
         });
 
         it('converts SVG to React', function() {
             var svg = data.svg.complex;
             var reactElement = Parser(svg);
-            assert.equal(render(reactElement), svg);
+            assert.equal(helpers.render(reactElement), svg);
         });
 
     });
@@ -87,15 +77,15 @@ describe('html-to-react', function() {
             it('overrides the element if replace is valid', function() {
                 var html = data.html.complex;
                 var reactElement = Parser(html, {
-                    replace: function(node) {
+                    replace: function(node, key) {
                         if (node.name === 'title') {
-                            return React.createElement('meta', { charSet: 'utf-8' });
+                            return React.createElement('title', { key: key }, 'Replaced Title');
                         }
                     }
                 });
                 assert.equal(
-                    render(reactElement),
-                    html.replace('<title>Title</title>', '<meta charset="utf-8"/>')
+                    helpers.render(reactElement),
+                    html.replace('<title>Title</title>', '<title>Replaced Title</title>')
                 );
             });
 
@@ -112,7 +102,7 @@ describe('html-to-react', function() {
                     }
                 });
                 assert.notEqual(
-                    render(reactElement),
+                    helpers.render(reactElement),
                     html.replace(
                         '<header id="header">Header</header>',
                         '<h1>Heading</h1>'
