Deprecate PromiseProxyMixin, Enumerable, and Observable (mixins, RFC#… - #21588
NullVoxPopuli wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
this is weird, sure, but also trying to deprecate all this stuff, while heavily used internally, and not throwing deprecations on it is difficult -- I'm super looking forward to the cleanup for v8
📊 Size reportTarball size — dist/dev 0.6%↑
Show files (29 files)
dist/prod 0.6%↑
Show files (29 files)
smoke-tests/v2-app-template/dist 1%↑
smoke-tests/v2-app-hello-world-template/dist -1.87%↓
🤖 This report was automatically generated by wyvox/pkg-size |
42d121c to
2271ebc
Compare
| class EmberObject extends CoreObject { | ||
| static { | ||
| // `extend(Observable)` would fire the Observable deprecation for every app. | ||
| this.PrototypeMixin.reopen(Observable); |
There was a problem hiding this comment.
see note about weirdness here: https://github.com/emberjs/ember.js/pull/21588/changes#diff-a717c1b4c9ec49161fcdbe9544aba7640f2bfebc3c924e980d4645927c29da0c
d6f0090 to
075a2b2
Compare
There was a problem hiding this comment.
i'm 🤷 on if these should us the public copies or not
There was a problem hiding this comment.
we will be deleting the public code ahead of when we cleanup our own code, so perhaps the way this is is better
There was a problem hiding this comment.
for how big this change, I 100% think we should rely on the existing tests as much as we can
| send(actionName: string, ...args: unknown[]): void; | ||
| } | ||
| const ActionSupport = Mixin[INTERNAL_MIXIN_CREATE]({ | ||
| send(actionName: string, ...args: unknown[]) { |
There was a problem hiding this comment.
this deprecation does still exist in the internal copy
There was a problem hiding this comment.
I didn't want to expose all these new internal files as importable things, so they got filtered out
075a2b2 to
2e69858
Compare
There was a problem hiding this comment.
updated to 7.5
| @property content | ||
| @type {unknown} | ||
| @default null | ||
| @public |
| } | ||
|
|
||
| const ProxyMixin = /*@__PURE__*/ Mixin[INTERNAL_MIXIN_CREATE]({ | ||
| const ProxyMixin = /*@__PURE__*/ DeprecatedMixin.create(InternalProxyMixin, { |
There was a problem hiding this comment.
Prob want to add @deprecated to the yuidoc above
| init() { | ||
| this._super(...arguments); | ||
| deprecateUntil( | ||
| 'The `ActionHandler` mixin is deprecated. Use the `@action` decorator and direct method calls instead.', |
There was a problem hiding this comment.
I think we already handled this with TargetActionSupport deprecation and it is private above so it is possible this can be until 7.9.0 (through one LTS)
| return this.__container__.factoryFor(fullName); | ||
| init() { | ||
| this._super(...arguments); | ||
| deprecateUntil( |
There was a problem hiding this comment.
Again since this was private the deprecation can be until 7.9
| const RegistryProxyMixin = DeprecatedMixin.create(InternalRegistryProxyMixin, { | ||
| init() { | ||
| this._super(...arguments); | ||
| deprecateUntil( |
There was a problem hiding this comment.
another that can be until 7.9
sorry for doing all as comments but github is being so slow I don't trust review
| @static | ||
| @for @ember/array | ||
| @return {Ember.NativeArray} | ||
| @public |
There was a problem hiding this comment.
Should this be marked public?
There was a problem hiding this comment.
it should not, thank you
| let value = get(controller, prop); | ||
| delegate(prop, value); | ||
| deprecateUntil( | ||
| 'The `ControllerMixin` mixin is deprecated. Extend `Controller` from `@ember/controller` instead.', |
There was a problem hiding this comment.
This mixin was private so the deprecation can be until 7.9
For the private mixins, if they weren't importable they don't even need deprecations
There was a problem hiding this comment.
they were all importable :(
| const Enumerable = DeprecatedMixin.create(InternalEnumerable, { | ||
| init() { | ||
| this._super(...arguments); | ||
| deprecateUntil( |
| init() { | ||
| this._super(...arguments); | ||
| deprecateUntil( | ||
| 'The `MutableEnumerable` mixin is deprecated. Use native arrays and array methods instead.', |
| init() { | ||
| this._super(...arguments); | ||
| deprecateUntil( | ||
| 'The `Evented` mixin is deprecated. Use native JavaScript events or a dedicated event library instead.', |
There was a problem hiding this comment.
All the methods already have deprecations and the docs are makred as @deprecated
There was a problem hiding this comment.
they did not log deprecations for deprecation-workflow tho
| @public | ||
| */ | ||
| static create<M extends typeof Mixin>(...args: any[]): InstanceType<M> { | ||
| static override create<M extends typeof InternalMixin>(...args: any[]): InstanceType<M> { |
There was a problem hiding this comment.
docs above need @deprecated
b19e196 to
27efb8f
Compare
RFC #1116 deprecates applying Ember's mixins, but Ember's own internals are built on several of them. Those mixins need to exist twice: an internal copy that applies silently, and a public copy that emits the deprecation. This is the mechanical half -- the six mixins under @ember/-internals move to `-internal` filenames with their contents unchanged, and their importers follow. The public, deprecating copies are added in the next commit. Add deprecating public mixin wrappers (RFC #1116 follow-up) Every mixin Ember applies to its own classes now exists twice: the internal copy applies silently, and the public copy wraps it and emits the deprecation from `init`. The public copies keep the API documentation; the internal copies carry the implementation. `Mixin` itself gets the same treatment -- `InternalMixin` holds the machinery and `Mixin.create` is the deprecating subclass -- which lets the `INTERNAL_MIXIN_CREATE` symbol go away entirely. Because the two copies are distinct objects, `meta.hasMixin` would no longer match a public mixin against an object that only ever had the internal copy applied. `DeprecatedMixin` overrides `detect` to look through to the wrapped copy, so `EmberArray.detect(someArrayProxy)` keeps working while the deprecation is live. Ember's own tests that exercise a mixin's behavior now apply the internal copy, matching what the framework does; tests that specifically cover the deprecation keep using the public copy. `EmberObject` already applies the internal `Observable`, so `observable_test` no longer re-applies the public one on top. The internal copies are kept out of `renamed-modules` so they are not advertised as importable module paths. Move deprecations to 7.5 Set deprecate flags, and update untils
27efb8f to
8ed880e
Compare
Supersedes:
Advancement:
"Deprecating Mixin Support"to Stage Ready for Release rfcs#1143RFC:
Guides:
Best viewed with:

Note
Because of the strategy of this deprecation (internal + public extends + deprecate), our tests don't have a lot of the testUnless stuff. We still need the tests because many of these mixins are still used internally. This makes this deprecation a bit different from the other ones due to how entangled mixins our in the codebase