Deprecate mixins - #21577
Conversation
📊 Size reportTarball size — dist/dev 0.1%↑
dist/prod 0.1%↑
smoke-tests/v2-app-template/dist 0.1%↑
smoke-tests/v2-app-hello-world-template/dist 1%↑
🤖 This report was automatically generated by wyvox/pkg-size |
762d617 to
fdb1a7a
Compare
| }); | ||
|
|
||
| let baseObject = MyMixin.apply({}); | ||
| let baseObject = mixin( |
There was a problem hiding this comment.
mixin was new to me, but it's a private thing for just us to use
| } | ||
|
|
||
| const ProxyMixin = /*@__PURE__*/ Mixin.create({ | ||
| const ProxyMixin = /*@__PURE__*/ Mixin[INTERNAL_MIXIN_CREATE]({ |
There was a problem hiding this comment.
this is the idea I had for allowing us to keep using mixin (until we get get rid of everything using it) but still throw deprecations on the public api mixin static methods
There was a problem hiding this comment.
Array deprecations coming soon
There was a problem hiding this comment.
(RFC already accepted)
| }, | ||
| }); | ||
|
|
||
| let BarControllerMixin = Mixin.create({ |
There was a problem hiding this comment.
mixin usage here was not relevant
There was a problem hiding this comment.
idk if we need this -- it's from a bot
There was a problem hiding this comment.
viewing this file requires hiding invis changes
| }, | ||
|
|
||
| class { | ||
| static module(title) { |
There was a problem hiding this comment.
this whole class can be deleted when we remove mixins, which will reduce the duplicate tests 🎉
| bar: 'BAR', | ||
| }); | ||
|
|
||
| let MixinB = MixinA.without('bar'); |
There was a problem hiding this comment.
I had forgotten about without
| }; | ||
|
|
||
| class B extends A.extend(Mixin1, Mixin2) { | ||
| class B extends A.extend(classicProps1, classicProps2) { |
There was a problem hiding this comment.
Shouldn't this also be deprecated?
There was a problem hiding this comment.
The RFC describes deprecating the mixin objects specifically: https://github.com/emberjs/rfcs/pull/1116/changes#diff-1d24a8c34da4efb2f7de310ceedabdaa99a19a8b37cda763044f741b2e172782R27
| this.add( | ||
| 'controller:parent', | ||
| Controller.extend(HasPage, { | ||
| Controller.extend(hasPage, { |
There was a problem hiding this comment.
What does passing multiple objects to extend mean if they aren't mixins?
There was a problem hiding this comment.
are plain objects mixins?
do we need a separate RFC for deprecating extend?
kategengler
left a comment
kategengler
left a comment
There was a problem hiding this comment.
Mostly 👍 but a couple of questions
2 participants
Advancement:
"Deprecating Mixin Support"to Stage Ready for Release rfcs#1143RFC:
The advancement RFC gives a bunch of other mixin-using things that are public that also need to be deprecated, but I'd like to that as a followup -- there are a bunch more things that need to be deprecated as a part of this RFC
In particular, these are not deprecated in this PR:
but will be soon (as the RFC declared they would be)