From ce5b15659c9539840b8eeb237e9e266d509f0d43 Mon Sep 17 00:00:00 2001 From: Hubert Olender Date: Sun, 24 May 2026 15:43:39 +0200 Subject: [PATCH] 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 --- .../@ember/-internals/deprecations/index.ts | 7 ++++++ .../runtime/lib/mixins/comparable.ts | 9 +++++++ .../runtime/tests/mixins/comparable_test.js | 11 ++++++-- packages/@ember/utils/lib/compare.ts | 25 ++++++++++++++++--- packages/@ember/utils/tests/compare_test.js | 10 ++++++-- 5 files changed, 55 insertions(+), 7 deletions(-) diff --git a/packages/@ember/-internals/deprecations/index.ts b/packages/@ember/-internals/deprecations/index.ts index bf105825bae..3fdf671569b 100644 --- a/packages/@ember/-internals/deprecations/index.ts +++ b/packages/@ember/-internals/deprecations/index.ts @@ -112,6 +112,13 @@ export const DEPRECATIONS = { until: '7.0.0', url: 'https://deprecations.emberjs.com/id/importing-inject-from-ember-service', }), + DEPRECATE_COMPARABLE_MIXIN: deprecation({ + for: 'ember-source', + id: 'deprecate-comparable-mixin', + since: { available: '7.2.0', enabled: '7.2.0' }, + until: '8.0.0', + url: 'https://deprecations.emberjs.com/id/deprecate-comparable-mixin', + }), }; export function deprecateUntil(message: string, deprecation: DeprecationObject) { diff --git a/packages/@ember/-internals/runtime/lib/mixins/comparable.ts b/packages/@ember/-internals/runtime/lib/mixins/comparable.ts index 451fd1d4885..e0b10e6eada 100644 --- a/packages/@ember/-internals/runtime/lib/mixins/comparable.ts +++ b/packages/@ember/-internals/runtime/lib/mixins/comparable.ts @@ -1,4 +1,5 @@ import Mixin from '@ember/object/mixin'; +import { deprecateUntil, DEPRECATIONS } from '@ember/-internals/deprecations'; /** @module ember @@ -37,6 +38,14 @@ const Comparable = Mixin.create({ @return {Number} the result of the comparison @private */ + init() { + this._super(...arguments); + deprecateUntil( + 'The `Comparable` mixin is deprecated. Implement a `compare` method directly on your class instead.', + DEPRECATIONS.DEPRECATE_COMPARABLE_MIXIN + ); + }, + compare: null, }); diff --git a/packages/@ember/-internals/runtime/tests/mixins/comparable_test.js b/packages/@ember/-internals/runtime/tests/mixins/comparable_test.js index 14920e423b2..edf437836dd 100644 --- a/packages/@ember/-internals/runtime/tests/mixins/comparable_test.js +++ b/packages/@ember/-internals/runtime/tests/mixins/comparable_test.js @@ -1,7 +1,8 @@ import EmberObject, { get } from '@ember/object'; import { compare } from '@ember/utils'; import Comparable from '../../lib/mixins/comparable'; -import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; +import { moduleFor, AbstractTestCase, expectDeprecation, testUnless } from 'internal-test-helpers'; +import { DEPRECATIONS } from '../../../deprecations'; class Rectangle extends EmberObject.extend(Comparable) { length = 0; @@ -22,11 +23,17 @@ moduleFor( 'Comparable', class extends AbstractTestCase { beforeEach() { + expectDeprecation( + /The `Comparable` mixin is deprecated/, + DEPRECATIONS.DEPRECATE_COMPARABLE_MIXIN.isEnabled + ); r1 = Rectangle.create({ length: 6, width: 12 }); r2 = Rectangle.create({ length: 6, width: 13 }); } - ['@test should be comparable and return the correct result'](assert) { + [`${testUnless( + DEPRECATIONS.DEPRECATE_COMPARABLE_MIXIN.isRemoved + )} @test should be comparable and return the correct result`](assert) { assert.equal(Comparable.detect(r1), true); assert.equal(compare(r1, r1), 0); assert.equal(compare(r1, r2), -1); diff --git a/packages/@ember/utils/lib/compare.ts b/packages/@ember/utils/lib/compare.ts index 26dae8d10ea..41aedf0999e 100644 --- a/packages/@ember/utils/lib/compare.ts +++ b/packages/@ember/utils/lib/compare.ts @@ -1,6 +1,5 @@ import type { TypeName } from './type-of'; import typeOf from './type-of'; -import Comparable from '@ember/-internals/runtime/lib/mixins/comparable'; import { assert } from '@ember/debug'; const TYPE_ORDER: Record = { @@ -163,10 +162,30 @@ export default function compare(v: T, w: T): Compare { } } +interface Comparable { + compare: (a: unknown, b: unknown) => -1 | 0 | 1; +} + interface ComparableConstructor { - constructor: Comparable; + constructor: { + compare: (a: unknown, b: unknown) => -1 | 0 | 1; + }; } function isComparable(value: unknown): value is Comparable & ComparableConstructor { - return Comparable.detect(value); + if (typeof value !== 'object' || value === null) { + return false; + } + + let maybeComparable = value as { + compare?: unknown; + constructor?: { + compare?: unknown; + }; + }; + + return ( + typeof maybeComparable.constructor?.compare === 'function' || + typeof maybeComparable.compare === 'function' + ); } diff --git a/packages/@ember/utils/tests/compare_test.js b/packages/@ember/utils/tests/compare_test.js index 9be6181a23f..c07be94af20 100644 --- a/packages/@ember/utils/tests/compare_test.js +++ b/packages/@ember/utils/tests/compare_test.js @@ -1,10 +1,9 @@ import { compare, typeOf } from '@ember/utils'; import EmberObject from '@ember/object'; -import { Comparable } from '@ember/-internals/runtime'; import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; let data = []; -let Comp = EmberObject.extend(Comparable); +let Comp = EmberObject.extend(); Comp.reopenClass({ compare(obj) { @@ -84,5 +83,12 @@ moduleFor( assert.equal(compare('b', zero), 0, 'Second item comparable - returns 0 (negated)'); assert.equal(compare('c', one), -1, 'Second item comparable - returns 1 (negated)'); } + + ['@test non-function compare does not make an object comparable'](assert) { + let obj = EmberObject.create({ compare: null }); + let other = EmberObject.create({ compare: 'not a function' }); + + assert.equal(compare(obj, other), 0, 'objects with non-function compare are not comparable'); + } } );