From a9ee31247928d5854226de3b9fe2ae67cea231ce Mon Sep 17 00:00:00 2001
From: Armand Abric <armand@forgebinaire.net>
Date: Sun, 12 May 2019 13:59:12 +0200
Subject: [PATCH] fix: Rework the propNameSorter to be less dependents of node
 sort internals

---
 src/formatter/formatReactElementNode.js |  4 ++--
 src/formatter/propNameSorter.js         | 23 ---------------------
 src/formatter/propNameSorter.spec.js    | 27 -------------------------
 src/formatter/sortPropsByNames.js       | 26 ++++++++++++++++++++++++
 src/formatter/sortPropsByNames.spec.js  | 27 +++++++++++++++++++++++++
 5 files changed, 55 insertions(+), 52 deletions(-)
 delete mode 100644 src/formatter/propNameSorter.js
 delete mode 100644 src/formatter/propNameSorter.spec.js
 create mode 100644 src/formatter/sortPropsByNames.js
 create mode 100644 src/formatter/sortPropsByNames.spec.js

diff --git a/src/formatter/formatReactElementNode.js b/src/formatter/formatReactElementNode.js
index 7c040ecd0..f21b0b2e4 100644
--- a/src/formatter/formatReactElementNode.js
+++ b/src/formatter/formatReactElementNode.js
@@ -4,7 +4,7 @@ import spacer from './spacer';
 import formatTreeNode from './formatTreeNode';
 import formatProp from './formatProp';
 import mergeSiblingPlainStringChildrenReducer from './mergeSiblingPlainStringChildrenReducer';
-import propNameSorter from './propNameSorter';
+import sortPropsByNames from './sortPropsByNames';
 import type { Options } from './../options';
 import type { ReactElementTreeNode } from './../tree';
 
@@ -137,7 +137,7 @@ export default (
     .filter(defaultPropName => !visibleAttributeNames.includes(defaultPropName))
     .forEach(defaultPropName => visibleAttributeNames.push(defaultPropName));
 
-  const attributes = visibleAttributeNames.sort(propNameSorter(sortProps));
+  const attributes = sortPropsByNames(sortProps)(visibleAttributeNames);
 
   attributes.forEach(attributeName => {
     const {
diff --git a/src/formatter/propNameSorter.js b/src/formatter/propNameSorter.js
deleted file mode 100644
index 4cd026140..000000000
--- a/src/formatter/propNameSorter.js
+++ /dev/null
@@ -1,23 +0,0 @@
-/* @flow */
-
-const isKeyOrRefProps = (propName: string) => ['key', 'ref'].includes(propName);
-
-export default (sortProps: boolean) => (a: string, b: string): -1 | 0 | 1 => {
-  if (a === b) {
-    return 0;
-  }
-
-  if (isKeyOrRefProps(a) && isKeyOrRefProps(b)) {
-    return 1;
-  } else if (isKeyOrRefProps(a)) {
-    return -1;
-  } else if (isKeyOrRefProps(b)) {
-    return 1;
-  }
-
-  if (!sortProps) {
-    return 0;
-  }
-
-  return a < b ? -1 : 1;
-};
diff --git a/src/formatter/propNameSorter.spec.js b/src/formatter/propNameSorter.spec.js
deleted file mode 100644
index c4686aefb..000000000
--- a/src/formatter/propNameSorter.spec.js
+++ /dev/null
@@ -1,27 +0,0 @@
-/* @flow */
-
-import propNameSorter from './propNameSorter';
-
-test('The propNameSorter should always move the `key` and `ref` keys first', () => {
-  const fixtures = ['c', 'key', 'a', 'ref', 'b'];
-
-  expect(fixtures.sort(propNameSorter(false))).toEqual([
-    'key',
-    'ref',
-    'c',
-    'a',
-    'b',
-  ]);
-});
-
-test('The propNameSorter should always sort the props and keep `key` and `ref` keys first', () => {
-  const fixtures = ['c', 'key', 'a', 'ref', 'b'];
-
-  expect(fixtures.sort(propNameSorter(true))).toEqual([
-    'key',
-    'ref',
-    'a',
-    'b',
-    'c',
-  ]);
-});
diff --git a/src/formatter/sortPropsByNames.js b/src/formatter/sortPropsByNames.js
new file mode 100644
index 000000000..649fd73bf
--- /dev/null
+++ b/src/formatter/sortPropsByNames.js
@@ -0,0 +1,26 @@
+/* @flow */
+
+const isKeyOrRefProps = (propName: string) => ['key', 'ref'].includes(propName);
+
+export default (shouldSortUserProps: boolean) => (
+  props: string[]
+): string[] => {
+  const haveKeyProp = props.includes('key');
+  const haveRefProp = props.includes('ref');
+
+  const userPropsOnly = props.filter(oneProp => !isKeyOrRefProps(oneProp));
+
+  const sortedProps = shouldSortUserProps
+    ? [...userPropsOnly.sort()] // We use basic lexical order
+    : [...userPropsOnly];
+
+  if (haveRefProp) {
+    sortedProps.unshift('ref');
+  }
+
+  if (haveKeyProp) {
+    sortedProps.unshift('key');
+  }
+
+  return sortedProps;
+};
diff --git a/src/formatter/sortPropsByNames.spec.js b/src/formatter/sortPropsByNames.spec.js
new file mode 100644
index 000000000..f77144be2
--- /dev/null
+++ b/src/formatter/sortPropsByNames.spec.js
@@ -0,0 +1,27 @@
+/* @flow */
+
+import sortPropsByNames from './sortPropsByNames';
+
+test('sortPropsByNames should always move the `key` and `ref` keys first', () => {
+  const fixtures = ['c', 'key', 'a', 'ref', 'b'];
+
+  expect(sortPropsByNames(false)(fixtures)).toEqual([
+    'key',
+    'ref',
+    'c',
+    'a',
+    'b',
+  ]);
+});
+
+test('sortPropsByNames should always sort the props and keep `key` and `ref` keys first', () => {
+  const fixtures = ['c', 'key', 'a', 'ref', 'b'];
+
+  expect(sortPropsByNames(true)(fixtures)).toEqual([
+    'key',
+    'ref',
+    'a',
+    'b',
+    'c',
+  ]);
+});