Skip to content

[CLEANUP] Deprecate Comparable mixin and remove internal usage#21437

Open
olenderhub wants to merge 1 commit into
emberjs:mainfrom
olenderhub:remove-comparable-mixin
Open

[CLEANUP] Deprecate Comparable mixin and remove internal usage#21437
olenderhub wants to merge 1 commit into
emberjs:mainfrom
olenderhub:remove-comparable-mixin

Conversation

@olenderhub

@olenderhub olenderhub commented May 29, 2026

Copy link
Copy Markdown
Contributor

Deprecate the legacy Comparable mixin while keeping it available until v8.

The internal compare() implementation no longer depends on Comparable.detect().
Instead, it uses a local duck-typing check for a function-valued compare method,
so Ember internals do not trigger the Comparable deprecation themselves.

  • Add a deprecation for the Comparable mixin
  • Keep the Comparable export and tree-shakable entrypoint available until v8
  • Update compare() to use duck-typing instead of Comparable.detect()
  • Add regression coverage for non-function compare values
  • Update Comparable tests to expect the deprecation

References:

@olenderhub olenderhub changed the title refactor: remove private Comparable mixin in favor of duck-type check [BUGFIX] Remove private Comparable mixin in favor of duck-type check May 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this cant be deleted until after a major after a deprecation lands -- like:

https://github.com/emberjs/ember.js/pull/20702/changes#diff-aa3f9176575f028f979266539b2a7647d98ec70c72162bd89de74f863c857f8b

and then at the major we can remove like here:

#20891

Comment thread packages/@ember/utils/tests/compare_test.js
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
@olenderhub olenderhub force-pushed the remove-comparable-mixin branch from 552d006 to ce5b156 Compare May 30, 2026 00:33
@olenderhub olenderhub changed the title [BUGFIX] Remove private Comparable mixin in favor of duck-type check [DEPRECATION] Deprecate Comparable mixin May 30, 2026
@olenderhub olenderhub changed the title [DEPRECATION] Deprecate Comparable mixin [CLEANUP] Deprecate Comparable mixin and remove internal usage May 30, 2026
@olenderhub

Copy link
Copy Markdown
Contributor Author

@NullVoxPopuli please check changes now. Is this is proper flow of this or I have to change something?

@NullVoxPopuli

Copy link
Copy Markdown
Contributor

looks, good I think. Do you have a PR ready for the deprecation guides as well?

@return {Number} the result of the comparison
@private
*/
init() {

@kategengler kategengler Jun 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NullVoxPopuli This mixin is marked @private -- do we believe it was public or intimate enough to need the full deprecation? (if not we could make the deprecation until 7.5 (until after next LTS))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm between the two -- based on https://emberobserver.com/code-search?codeQuery=Comparable

but one of those libraries is quite popular, and has a 10, but I think its users know that it's a library they should move away from.

I can always lean towards removing stuff sooner, since the community has had a collective dislike for mixins since i started ember

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think model fragments is the only legit one there but also it says private and probably has for a long time ...

I think we can change the until to be 7.5

@olenderhub

Copy link
Copy Markdown
Contributor Author

looks, good I think. Do you have a PR ready for the deprecation guides as well?

@NullVoxPopuli Created ember-learn/deprecation-app#1432.

I kept the versions matching this PR for now, but can update if you decide on the shorter timeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants