From 98ec724a23311e9cf91c785086a2f96c2487d04b Mon Sep 17 00:00:00 2001
From: Lee Byron <lee.byron@robinhood.com>
Date: Tue, 18 Sep 2018 20:20:38 -0700
Subject: [PATCH] [BREAKING] Move sort() to ordered collections

Map.sort() returns an OrderedMap (same for Set and OrderedSet) which creates a circular dependency between them and makes it impossible to tree shake away the ordered variants.

WORK IN PROGRESS:
* Move the sort() and sortBy() dts and flow to Seq, List, Ordered*, Stack?
* Move generic definition on CollectionImpl to each usage
* Use method/function pattern?
---
 src/Map.js        | 13 -------------
 src/OrderedMap.js | 11 +++++++++++
 src/OrderedSet.js | 11 +++++++++++
 src/Set.js        | 13 -------------
 4 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/src/Map.js b/src/Map.js
index 9ff3f6db26..2ace229933 100644
--- a/src/Map.js
+++ b/src/Map.js
@@ -21,7 +21,6 @@ import {
 } from './TrieUtils';
 import { hash } from './Hash';
 import { Iterator, iteratorValue, iteratorDone } from './Iterator';
-import { sortFactory } from './Operations';
 import arrCopy from './utils/arrCopy';
 import assertNotInfinite from './utils/assertNotInfinite';
 import { setIn } from './methods/setIn';
@@ -37,8 +36,6 @@ import { asMutable } from './methods/asMutable';
 import { asImmutable } from './methods/asImmutable';
 import { wasAltered } from './methods/wasAltered';
 
-import { OrderedMap } from './OrderedMap';
-
 export class Map extends KeyedCollection {
   // @pragma Construction
 
@@ -115,16 +112,6 @@ export class Map extends KeyedCollection {
 
   // @pragma Composition
 
-  sort(comparator) {
-    // Late binding
-    return OrderedMap(sortFactory(this, comparator));
-  }
-
-  sortBy(mapper, comparator) {
-    // Late binding
-    return OrderedMap(sortFactory(this, comparator, mapper));
-  }
-
   map(mapper, context) {
     return this.withMutations(map => {
       map.forEach((value, key) => {
diff --git a/src/OrderedMap.js b/src/OrderedMap.js
index 6e6b769d48..6ccf146209 100644
--- a/src/OrderedMap.js
+++ b/src/OrderedMap.js
@@ -10,6 +10,7 @@ import { IS_ORDERED_SYMBOL } from './predicates/isOrdered';
 import { isOrderedMap } from './predicates/isOrderedMap';
 import { Map, emptyMap } from './Map';
 import { emptyList } from './List';
+import { reify, sortFactory } from './Operations';
 import { DELETE, NOT_SET, SIZE } from './TrieUtils';
 import assertNotInfinite from './utils/assertNotInfinite';
 
@@ -66,6 +67,16 @@ export class OrderedMap extends Map {
     return updateOrderedMap(this, k, NOT_SET);
   }
 
+  // @pragma Composition
+
+  sort(comparator) {
+    return reify(this, sortFactory(this, comparator));
+  }
+
+  sortBy(mapper, comparator) {
+    return reify(this, sortFactory(this, comparator, mapper));
+  }
+
   wasAltered() {
     return this._map.wasAltered() || this._list.wasAltered();
   }
diff --git a/src/OrderedSet.js b/src/OrderedSet.js
index a4d27d0871..730ed72ebe 100644
--- a/src/OrderedSet.js
+++ b/src/OrderedSet.js
@@ -10,6 +10,7 @@ import { IS_ORDERED_SYMBOL } from './predicates/isOrdered';
 import { isOrderedSet } from './predicates/isOrderedSet';
 import { IndexedCollectionPrototype } from './CollectionImpl';
 import { Set } from './Set';
+import { reify, sortFactory } from './Operations';
 import { emptyOrderedMap } from './OrderedMap';
 import assertNotInfinite from './utils/assertNotInfinite';
 
@@ -36,6 +37,16 @@ export class OrderedSet extends Set {
     return this(KeyedCollection(value).keySeq());
   }
 
+  // Composition
+
+  sort(comparator) {
+    return reify(this, sortFactory(this, comparator));
+  }
+
+  sortBy(mapper, comparator) {
+    return reify(this, sortFactory(this, comparator, mapper));
+  }
+
   toString() {
     return this.__toString('OrderedSet {', '}');
   }
diff --git a/src/Set.js b/src/Set.js
index 1242052b46..0ee151a41c 100644
--- a/src/Set.js
+++ b/src/Set.js
@@ -10,14 +10,11 @@ import { isOrdered } from './predicates/isOrdered';
 import { IS_SET_SYMBOL, isSet } from './predicates/isSet';
 import { emptyMap } from './Map';
 import { DELETE } from './TrieUtils';
-import { sortFactory } from './Operations';
 import assertNotInfinite from './utils/assertNotInfinite';
 import { asImmutable } from './methods/asImmutable';
 import { asMutable } from './methods/asMutable';
 import { withMutations } from './methods/withMutations';
 
-import { OrderedSet } from './OrderedSet';
-
 export class Set extends SetCollection {
   // @pragma Construction
 
@@ -136,16 +133,6 @@ export class Set extends SetCollection {
     });
   }
 
-  sort(comparator) {
-    // Late binding
-    return OrderedSet(sortFactory(this, comparator));
-  }
-
-  sortBy(mapper, comparator) {
-    // Late binding
-    return OrderedSet(sortFactory(this, comparator, mapper));
-  }
-
   wasAltered() {
     return this._map.wasAltered();
   }