Skip to content

Commit 552d006

Browse files
committed
refactor: remove private Comparable mixin in favor of duck-type check
Remove the legacy Comparable mixin (Mixin.create) and replace its only usage in compare() with a local isComparable function that checks whether compare is a function via duck-typing. - Delete packages/@ember/-internals/runtime/lib/mixins/comparable.ts - Delete comparable_test.js - Remove Comparable export from runtime/index.ts - Clean up stale references in lib/index.js, package.json, and tree-shakability test snapshots - Add regression test for non-function compare values
1 parent 2af952a commit 552d006

8 files changed

Lines changed: 30 additions & 89 deletions

File tree

lib/index.cjs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ const shim = addonV1Shim(path.join(__dirname, '..'), {
4141
'./dist/dev/packages/@ember/-internals/runtime/index.js',
4242
'./dist/dev/packages/@ember/-internals/runtime/lib/ext/rsvp.js',
4343
'./dist/dev/packages/@ember/-internals/runtime/lib/mixins/-proxy.js',
44-
'./dist/dev/packages/@ember/-internals/runtime/lib/mixins/comparable.js',
4544
'./dist/dev/packages/@ember/-internals/string/index.js',
4645
'./dist/dev/packages/@ember/-internals/utility-types/index.js',
4746
'./dist/dev/packages/@ember/-internals/utils/index.js',

package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,6 @@
190190
"@ember/-internals/runtime/lib/ext/rsvp.js": "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js",
191191
"@ember/-internals/runtime/lib/mixins/-proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/-proxy.js",
192192
"@ember/-internals/runtime/lib/mixins/action_handler.js": "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js",
193-
"@ember/-internals/runtime/lib/mixins/comparable.js": "ember-source/@ember/-internals/runtime/lib/mixins/comparable.js",
194193
"@ember/-internals/runtime/lib/mixins/container_proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js",
195194
"@ember/-internals/runtime/lib/mixins/registry_proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js",
196195
"@ember/-internals/runtime/lib/mixins/target_action_support.js": "ember-source/@ember/-internals/runtime/lib/mixins/target_action_support.js",

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
export { default as RegistryProxyMixin } from './lib/mixins/registry_proxy';
22
export { default as ContainerProxyMixin } from './lib/mixins/container_proxy';
3-
export { default as Comparable } from './lib/mixins/comparable';
43
export { default as ActionHandler } from './lib/mixins/action_handler';
54
export { default as _ProxyMixin, contentFor as _contentFor } from './lib/mixins/-proxy';
65
export { default as MutableEnumerable } from '@ember/enumerable/mutable';

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

Lines changed: 0 additions & 43 deletions
This file was deleted.

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

Lines changed: 0 additions & 36 deletions
This file was deleted.

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
);

tests/node-vitest/tree-shakability.test.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ it('[dev] has expected tree-shakable entrypoints', async () => {
115115
"ember-source/@ember/-internals/runtime/lib/ext/rsvp.js",
116116
"ember-source/@ember/-internals/runtime/lib/mixins/-proxy.js",
117117
"ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js",
118-
"ember-source/@ember/-internals/runtime/lib/mixins/comparable.js",
119118
"ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js",
120119
"ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js",
121120
"ember-source/@ember/-internals/runtime/lib/mixins/target_action_support.js",
@@ -319,7 +318,6 @@ it('[prod] has expected tree-shakable entrypoints', async () => {
319318
"ember-source/@ember/-internals/runtime/lib/ext/rsvp.js",
320319
"ember-source/@ember/-internals/runtime/lib/mixins/-proxy.js",
321320
"ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js",
322-
"ember-source/@ember/-internals/runtime/lib/mixins/comparable.js",
323321
"ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js",
324322
"ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js",
325323
"ember-source/@ember/-internals/runtime/lib/mixins/target_action_support.js",

0 commit comments

Comments
 (0)