-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
[CLEANUP] Deprecate Comparable mixin and remove internal usage #21437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @NullVoxPopuli This mixin is marked
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 I think we can change the until to be 7.5 |
||
| this._super(...arguments); | ||
| deprecateUntil( | ||
| 'The `Comparable` mixin is deprecated. Implement a `compare` method directly on your class instead.', | ||
| DEPRECATIONS.DEPRECATE_COMPARABLE_MIXIN | ||
| ); | ||
| }, | ||
|
|
||
| compare: null, | ||
| }); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
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