Skip to content

Commit ce5b156

Browse files
committed
deprecate Comparable mixin and remove internal usage
Keep the Comparable mixin available as deprecated API until v8, while removing Ember's internal dependency on it from compare(). This allows external apps/addons using Comparable to receive a deprecation warning without Ember internals triggering the deprecation themselves. - Add a deprecation for Comparable - Preserve the Comparable mixin API until v8 - Update compare() to use duck-typing instead of Comparable.detect() - Add regression coverage for non-function compare values
1 parent 2af952a commit ce5b156

5 files changed

Lines changed: 55 additions & 7 deletions

File tree

packages/@ember/-internals/deprecations/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,13 @@ export const DEPRECATIONS = {
112112
until: '7.0.0',
113113
url: 'https://deprecations.emberjs.com/id/importing-inject-from-ember-service',
114114
}),
115+
DEPRECATE_COMPARABLE_MIXIN: deprecation({
116+
for: 'ember-source',
117+
id: 'deprecate-comparable-mixin',
118+
since: { available: '7.2.0', enabled: '7.2.0' },
119+
until: '8.0.0',
120+
url: 'https://deprecations.emberjs.com/id/deprecate-comparable-mixin',
121+
}),
115122
};
116123

117124
export function deprecateUntil(message: string, deprecation: DeprecationObject) {

packages/@ember/-internals/runtime/lib/mixins/comparable.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import Mixin from '@ember/object/mixin';
2+
import { deprecateUntil, DEPRECATIONS } from '@ember/-internals/deprecations';
23

34
/**
45
@module ember
@@ -37,6 +38,14 @@ const Comparable = Mixin.create({
3738
@return {Number} the result of the comparison
3839
@private
3940
*/
41+
init() {
42+
this._super(...arguments);
43+
deprecateUntil(
44+
'The `Comparable` mixin is deprecated. Implement a `compare` method directly on your class instead.',
45+
DEPRECATIONS.DEPRECATE_COMPARABLE_MIXIN
46+
);
47+
},
48+
4049
compare: null,
4150
});
4251

packages/@ember/-internals/runtime/tests/mixins/comparable_test.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import EmberObject, { get } from '@ember/object';
22
import { compare } from '@ember/utils';
33
import Comparable from '../../lib/mixins/comparable';
4-
import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
4+
import { moduleFor, AbstractTestCase, expectDeprecation, testUnless } from 'internal-test-helpers';
5+
import { DEPRECATIONS } from '../../../deprecations';
56

67
class Rectangle extends EmberObject.extend(Comparable) {
78
length = 0;
@@ -22,11 +23,17 @@ moduleFor(
2223
'Comparable',
2324
class extends AbstractTestCase {
2425
beforeEach() {
26+
expectDeprecation(
27+
/The `Comparable` mixin is deprecated/,
28+
DEPRECATIONS.DEPRECATE_COMPARABLE_MIXIN.isEnabled
29+
);
2530
r1 = Rectangle.create({ length: 6, width: 12 });
2631
r2 = Rectangle.create({ length: 6, width: 13 });
2732
}
2833

29-
['@test should be comparable and return the correct result'](assert) {
34+
[`${testUnless(
35+
DEPRECATIONS.DEPRECATE_COMPARABLE_MIXIN.isRemoved
36+
)} @test should be comparable and return the correct result`](assert) {
3037
assert.equal(Comparable.detect(r1), true);
3138
assert.equal(compare(r1, r1), 0);
3239
assert.equal(compare(r1, r2), -1);

packages/@ember/utils/lib/compare.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { TypeName } from './type-of';
22
import typeOf from './type-of';
3-
import Comparable from '@ember/-internals/runtime/lib/mixins/comparable';
43
import { assert } from '@ember/debug';
54

65
const TYPE_ORDER: Record<TypeName, number> = {
@@ -163,10 +162,30 @@ export default function compare<T>(v: T, w: T): Compare {
163162
}
164163
}
165164

165+
interface Comparable {
166+
compare: (a: unknown, b: unknown) => -1 | 0 | 1;
167+
}
168+
166169
interface ComparableConstructor {
167-
constructor: Comparable;
170+
constructor: {
171+
compare: (a: unknown, b: unknown) => -1 | 0 | 1;
172+
};
168173
}
169174

170175
function isComparable(value: unknown): value is Comparable & ComparableConstructor {
171-
return Comparable.detect(value);
176+
if (typeof value !== 'object' || value === null) {
177+
return false;
178+
}
179+
180+
let maybeComparable = value as {
181+
compare?: unknown;
182+
constructor?: {
183+
compare?: unknown;
184+
};
185+
};
186+
187+
return (
188+
typeof maybeComparable.constructor?.compare === 'function' ||
189+
typeof maybeComparable.compare === 'function'
190+
);
172191
}

packages/@ember/utils/tests/compare_test.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
import { compare, typeOf } from '@ember/utils';
22
import EmberObject from '@ember/object';
3-
import { Comparable } from '@ember/-internals/runtime';
43
import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
54

65
let data = [];
7-
let Comp = EmberObject.extend(Comparable);
6+
let Comp = EmberObject.extend();
87

98
Comp.reopenClass({
109
compare(obj) {
@@ -84,5 +83,12 @@ moduleFor(
8483
assert.equal(compare('b', zero), 0, 'Second item comparable - returns 0 (negated)');
8584
assert.equal(compare('c', one), -1, 'Second item comparable - returns 1 (negated)');
8685
}
86+
87+
['@test non-function compare does not make an object comparable'](assert) {
88+
let obj = EmberObject.create({ compare: null });
89+
let other = EmberObject.create({ compare: 'not a function' });
90+
91+
assert.equal(compare(obj, other), 0, 'objects with non-function compare are not comparable');
92+
}
8793
}
8894
);

0 commit comments

Comments
 (0)