Fix default sort comparator to logically handle null/undefined values (…

…#922)

* Fix default sort comparator to logically handle null/undefined values

* changelog

* Add unit test for sorting undefined values, modified docs data to include some empty values

* update default comparator test to include a null value

CHANGELOG.md

@@ -6,6 +6,7 @@
 **Bug fixes**
 
 - Added `role="dialog"` to `EuiFlyout` to improve screen reader accessibility ([#916](https://github.com/elastic/eui/pull/916))
+- Default sort comparator (used by `EuiInMemoryTable`) now handles `null` and `undefined` values ([#922](https://github.com/elastic/eui/pull/922))
 
 ## [`0.0.52`](https://github.com/elastic/eui/tree/v0.0.52)
 

src-docs/src/views/tables/data_store.js

@@ -32,7 +32,7 @@ const createCountries = () => [
 ];
 
 const firstNames = ['Very long first name that will wrap or be truncated', 'Another very long first name which will wrap or be truncated',
-  'Clinton', 'Igor', 'Karl', 'Drew', 'Honza', 'Rashid', 'Jordan', 'John'];
+  'Clinton', 'Igor', undefined, 'Drew', null, 'Rashid', undefined, 'John'];
 
 const lastNames = ['Very long last name that will wrap or be truncated', 'Another very long last name which will wrap or be truncated',
   'Gormley', 'Motov', 'Minarik', 'Raines', 'Král', 'Khan', 'Sissel', 'Dorlus'];

src/services/sort/comparators.js

@@ -5,6 +5,34 @@ export const Comparators = Object.freeze({
 
   default: (direction = SortDirection.ASC) => {
     return (v1, v2) => {
+      // JavaScript's comparison of null/undefined (and some others not handled here) values always returns `false`
+      // (https://www.ecma-international.org/ecma-262/#sec-abstract-relational-comparison)
+      // resulting in cases where v1 < v2 and v1 > v2 are both false.
+      // This leads to unpredictable sorting results in multiple JS engine sorting algorithms
+      // which causes unpredictable user experiences.
+      // Instead:
+      // * 1. push undefined/null values to the end of the sorted list, _regardless of sort direction_
+      //    (non-sortable values always appear at the end, and sortable values are sorted at the top)
+      // * 2. report undefined/null values as equal
+      // * 3. when both values are comparable they are sorted normally
+
+      const v1IsComparable = v1 != null;
+      const v2IsComparable = v2 != null;
+
+      // * 1.
+      if (v1IsComparable && !v2IsComparable) {
+        return -1;
+      }
+      if (!v1IsComparable && v2IsComparable) {
+        return 1;
+      }
+
+      // * 2.
+      if (!v1IsComparable && !v2IsComparable) {
+        return 0;
+      }
+
+      // * 3.
       if (v1 === v2) {
         return 0;
       }

src/services/sort/comparators.test.js

@@ -46,3 +46,10 @@ describe('comparators - property', () => {
   });
 });
 
+describe('default comparator', () => {
+  test('null/undefined values are sorted to the end', () => {
+    const sorted = [undefined, '7', 3, null, 5, undefined].sort(Comparators.default());
+    expect(sorted).toEqual([3, 5, '7', null, undefined, undefined])
+  })
+});
+