[Bugfix beta] fix Comparable.compare [fixes #5299 #5060]

emberjs · Nov 1, 2014 · 19be573 · 19be573
1 parent 6db5880
commit 19be573
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 15 deletions.
diff --git a/packages/ember-metal/lib/computed.js b/packages/ember-metal/lib/computed.js
@@ -1,4 +1,3 @@
-import Ember from "ember-metal/core";
 import { set } from "ember-metal/property_set";
 import {
   meta,

diff --git a/packages/ember-runtime/lib/compare.js b/packages/ember-runtime/lib/compare.js
@@ -55,16 +55,17 @@ export default function compare(v, w) {
   var type2 = typeOf(w);
 
   if (Comparable) {
-    if (type1 ==='instance' && Comparable.detect(v.constructor)) {
+    if (type1 === 'instance' && Comparable.detect(v) && v.constructor.compare) {
       return v.constructor.compare(v, w);
     }
 
-    if (type2 === 'instance' && Comparable.detect(w.constructor)) {
-      return 1 - w.constructor.compare(w, v);
+    if (type2 === 'instance' && Comparable.detect(w) && w.constructor.compare) {
+      return w.constructor.compare(w, v) * -1;
     }
   }
 
   var res = spaceship(TYPE_ORDER[type1], TYPE_ORDER[type2]);
+
   if (res !== 0) {
     return res;
   }

diff --git a/packages/ember-runtime/tests/core/compare_test.js b/packages/ember-runtime/tests/core/compare_test.js
@@ -4,9 +4,11 @@ import compare from 'ember-runtime/compare';
 import Comparable from 'ember-runtime/mixins/comparable';
 
 var data = [];
-var Comp = EmberObject.extend(Comparable, {
-  compare: function () {
-    return this.get('val');
+var Comp = EmberObject.extend(Comparable);
+
+Comp.reopenClass({
+  compare: function (obj) {
+    return obj.get('val');
   }
 });
 
@@ -53,15 +55,23 @@ test('ordering should work', function() {
 });
 
 test('comparables should return values in the range of -1, 0, 1', function() {
-  var negOne = Comp.create({ val: -1 });
-  var zero = Comp.create({ val: 0 });
-  var one = Comp.create({ val: 1 });
+  var negOne = Comp.create({
+    val: -1
+  });
+
+  var zero = Comp.create({
+    val: 0
+  });
+
+  var one = Comp.create({
+    val: 1
+  });
 
   equal(compare(negOne, 'a'), -1, 'First item comparable - returns -1 (not negated)');
-  equal(compare(zero, 'b'), 0, 'First item comparable - returns 0 (not negated)');
-  equal(compare(one, 'c'), 1, 'First item comparable - returns 1 (not negated)');
+  equal(compare(zero, 'b'),    0, 'First item comparable - returns  0 (not negated)');
+  equal(compare(one, 'c'),     1, 'First item comparable - returns  1 (not negated)');
 
-  equal(compare('a', negOne), 1, 'Second item comparable - returns -1 (negated)');
-  equal(compare('b', zero), 0, 'Second item comparable - returns 0 (negated)');
-  equal(compare('c', one), -1, 'Second item comparable - returns 1 (negated)');
+  equal(compare('a', negOne),  1, 'Second item comparable - returns -1 (negated)');
+  equal(compare('b', zero),    0, 'Second item comparable - returns  0 (negated)');
+  equal(compare('c', one),    -1, 'Second item comparable - returns  1 (negated)');
 });