Merge pull request #394 from vidhu/fix/trimming-issue

fix: trim only when necessary if trimming is off

Author: remarkablemark

README.md

@@ -50,7 +50,6 @@ parse('<p>Hello, World!</p>'); // React.createElement('p', {}, 'Hello, World!')
   - [Parser throws an error](#parser-throws-an-error)
   - [Is SSR supported?](#is-ssr-supported)
   - [Elements aren't nested correctly](#elements-arent-nested-correctly)
-  - [Warning: validateDOMNesting(...): Whitespace text nodes cannot appear as a child of table](#warning-validatedomnesting-whitespace-text-nodes-cannot-appear-as-a-child-of-table)
   - [Don't change case of tags](#dont-change-case-of-tags)
   - [TS Error: Property 'attribs' does not exist on type 'DOMNode'](#ts-error-property-attribs-does-not-exist-on-type-domnode)
   - [Can I enable `trim` for certain elements?](#can-i-enable-trim-for-certain-elements)
@@ -351,16 +350,16 @@ By default, whitespace is preserved:
 parse('<br>\n'); // [React.createElement('br'), '\n']
 ```
 
-To remove whitespace, enable the `trim` option:
+But certain elements like `<table>` will strip out invalid whitespace:
 
 ```js
-parse('<br>\n', { trim: true }); // React.createElement('br')
+parse('<table>\n</table>'); // React.createElement('table')
 ```
 
-This fixes the warning:
+To remove whitespace, enable the `trim` option:
 
-```
-Warning: validateDOMNesting(...): Whitespace text nodes cannot appear as a child of <table>. Make sure you don't have any extra whitespace between tags on each line of your source code.
+```js
+parse('<br>\n', { trim: true }); // React.createElement('br')
 ```
 
 However, intentional whitespace may be stripped out:
@@ -427,10 +426,6 @@ parse('<div /><div />'); // returns single element instead of array of elements
 
 See [#158](https://github.com/remarkablemark/html-react-parser/issues/158).
 
-### Warning: validateDOMNesting(...): Whitespace text nodes cannot appear as a child of table
-
-Enable the [trim](#trim) option. See [#155](https://github.com/remarkablemark/html-react-parser/issues/155).
-
 ### Don't change case of tags
 
 Tags are lowercased by default. To prevent that from happening, pass the [htmlparser2 option](#htmlparser2):

lib/dom-to-react.js

@@ -3,6 +3,7 @@ var attributesToProps = require('./attributes-to-props');
 var utilities = require('./utilities');
 
 var setStyleProp = utilities.setStyleProp;
+var canTextBeChildOfNode = utilities.canTextBeChildOfNode;
 
 /**
  * Converts DOM nodes to JSX element(s).
@@ -23,11 +24,11 @@ function domToReact(nodes, options) {
 
   var result = [];
   var node;
+  var isWhitespace;
   var hasReplace = typeof options.replace === 'function';
   var replaceElement;
   var props;
   var children;
-  var data;
   var trim = options.trim;
 
   for (var i = 0, len = nodes.length; i < len; i++) {
@@ -51,15 +52,23 @@ function domToReact(nodes, options) {
     }
 
     if (node.type === 'text') {
-      // if trim option is enabled, skip whitespace text nodes
-      if (trim) {
-        data = node.data.trim();
-        if (data) {
-          result.push(node.data);
-        }
-      } else {
-        result.push(node.data);
+      isWhitespace = !node.data.trim().length;
+
+      if (isWhitespace && node.parent && !canTextBeChildOfNode(node.parent)) {
+        // We have a whitespace node that can't be nested in its parent
+        // so skip it
+        continue;
       }
+
+      if (trim && isWhitespace) {
+        // Trim is enabled and we have a whitespace node
+        // so skip it
+        continue;
+      }
+
+      // We have a text node that's not whitespace and it can be nested
+      // in its parent so add it to the results
+      result.push(node.data);
       continue;
     }
 

lib/utilities.js

@@ -96,9 +96,35 @@ function setStyleProp(style, props) {
  */
 var PRESERVE_CUSTOM_ATTRIBUTES = React.version.split('.')[0] >= 16;
 
+// Taken from
+// https://github.com/facebook/react/blob/cae635054e17a6f107a39d328649137b83f25972/packages/react-dom/src/client/validateDOMNesting.js#L213
+var elementsWithNoTextChildren = new Set([
+  'tr',
+  'tbody',
+  'thead',
+  'tfoot',
+  'colgroup',
+  'table',
+  'head',
+  'html',
+  'frameset'
+]);
+
+/**
+ * Checks if the given node can contain text nodes
+ *
+ * @param {DomElement} node
+ * @returns {boolean}
+ */
+function canTextBeChildOfNode(node) {
+  return !elementsWithNoTextChildren.has(node.name);
+}
+
 module.exports = {
   PRESERVE_CUSTOM_ATTRIBUTES: PRESERVE_CUSTOM_ATTRIBUTES,
   invertObject: invertObject,
   isCustomComponent: isCustomComponent,
-  setStyleProp: setStyleProp
+  setStyleProp: setStyleProp,
+  canTextBeChildOfNode: canTextBeChildOfNode,
+  elementsWithNoTextChildren: elementsWithNoTextChildren
 };

test/index.test.js

@@ -138,13 +138,34 @@ describe('htmlparser2 option', () => {
 });
 
 describe('trim option', () => {
-  it('preserves whitespace text nodes when disabled (default)', () => {
+  it('preserves whitespace text nodes when disabled if valid in parent (default)', () => {
     const html = `<table>
   <tbody>
+    <tr><td>hello</td><td>\n</td><td>&nbsp;</td>\t</tr>\r
   </tbody>
 </table>`;
     const reactElement = parse(html);
-    expect(render(reactElement)).toBe(html);
+    expect(render(reactElement)).toBe(
+      '<table><tbody><tr><td>hello</td><td>\n</td><td>\u00a0</td></tr></tbody></table>'
+    );
+    expect(reactElement).toMatchInlineSnapshot(`
+      <table>
+        <tbody>
+          <tr>
+            <td>
+              hello
+            </td>
+            <td>
+              
+
+            </td>
+            <td>
+              \u00a0
+            </td>
+          </tr>
+        </tbody>
+      </table>
+    `);
   });
 
   it('removes whitespace text nodes when enabled', () => {

test/utilities.test.js

@@ -3,7 +3,9 @@ const {
   PRESERVE_CUSTOM_ATTRIBUTES,
   invertObject,
   isCustomComponent,
-  setStyleProp
+  setStyleProp,
+  elementsWithNoTextChildren,
+  canTextBeChildOfNode
 } = require('../lib/utilities');
 
 describe('invertObject', () => {
@@ -125,3 +127,22 @@ describe('setStyleProp', () => {
     expect(props).toEqual({ style: {} });
   });
 });
+
+describe('canTextBeChildOfNode', () => {
+  it.each(Array.from(elementsWithNoTextChildren))(
+    'returns false since text node cannot be child of %s',
+    nodeName => {
+      const node = {
+        name: nodeName
+      };
+      expect(canTextBeChildOfNode(node)).toBe(false);
+    }
+  );
+
+  it('returns true if text can be child of <td/>', () => {
+    const node = {
+      name: 'td'
+    };
+    expect(canTextBeChildOfNode(node)).toBe(true);
+  });
+});