fix(formatting): fix an edge case where number and string childrens a…

…re not correctly merged
algolia · Jul 21, 2017 · 47572e0 · 47572e0
1 parent b50fe85
commit 47572e0
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 23 deletions.
diff --git a/src/formatter/mergeSiblingPlainStringChildrenReducer.js b/src/formatter/mergeSiblingPlainStringChildrenReducer.js
@@ -2,27 +2,20 @@
 
 import type { TreeNode } from './../tree';
 
+const areMergeable = (nodeA: TreeNode, nodeB: TreeNode): boolean =>
+  (nodeA.type === 'string' || nodeA.type === 'number') &&
+  (nodeB.type === 'string' || nodeB.type === 'number');
+
 export default (
   previousNodes: TreeNode[],
   currentNode: TreeNode
 ): TreeNode[] => {
   const lastNode = previousNodes[previousNodes.length - 1];
-  const newNode = { ...currentNode };
-
-  if (newNode.type === 'number') {
-    newNode.type = 'string';
-    newNode.value = String(newNode.value);
-  }
 
-  if (
-    lastNode &&
-    lastNode.type === 'string' &&
-    typeof lastNode.value === 'string' &&
-    newNode.type === 'string'
-  ) {
-    lastNode.value += newNode.value || '';
+  if (lastNode && areMergeable(lastNode, currentNode)) {
+    lastNode.value = String(lastNode.value) + String(currentNode.value);
   } else {
-    previousNodes.push(newNode);
+    previousNodes.push(currentNode);
   }
 
   return previousNodes;

diff --git a/src/formatter/mergeSiblingPlainStringChildrenReducer.spec.js b/src/formatter/mergeSiblingPlainStringChildrenReducer.spec.js
@@ -24,22 +24,35 @@ test('mergeSiblingPlainStringChildrenReducer should merge sibling string tree no
   ]);
 });
 
-test('mergeSiblingPlainStringChildrenReducer should merge consider number as string', () => {
-  const childrens: TreeNode[] = [
-    createScalarTreeNode('string', 'a'),
-    createScalarTreeNode('number', 51),
-    createScalarTreeNode('string', 'c'),
-  ];
-
-  expect(childrens.reduce(mergeSiblingPlainStringChildrenReducer, [])).toEqual([
+test('mergeSiblingPlainStringChildrenReducer should consider number as string', () => {
+  expect(
+    [
+      createScalarTreeNode('string', 'a'),
+      createScalarTreeNode('number', 51),
+      createScalarTreeNode('string', 'c'),
+    ].reduce(mergeSiblingPlainStringChildrenReducer, [])
+  ).toEqual([
     {
       type: 'string',
       value: 'a51c',
     },
   ]);
+
+  expect(
+    [
+      createScalarTreeNode('string', 5),
+      createScalarTreeNode('number', 1),
+      createScalarTreeNode('string', 'a'),
+    ].reduce(mergeSiblingPlainStringChildrenReducer, [])
+  ).toEqual([
+    {
+      type: 'string',
+      value: '51a',
+    },
+  ]);
 });
 
-test('mergeSiblingPlainStringChildrenReducer should merge detect non string node', () => {
+test('mergeSiblingPlainStringChildrenReducer should detect non string node', () => {
   const childrens: TreeNode[] = [
     createReactElementTreeNode(['foo']),
     createScalarTreeNode('string', 'a'),