Skip to content

Mixin migration - WIP#21429

Open
olenderhub wants to merge 6 commits into
emberjs:mainfrom
olenderhub:mixin-migration
Open

Mixin migration - WIP#21429
olenderhub wants to merge 6 commits into
emberjs:mainfrom
olenderhub:mixin-migration

Conversation

@olenderhub
Copy link
Copy Markdown
Contributor

WIP

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
…r mixins

- Delete obsolete @ember/enumerable stub package (index.ts, mutable.ts,
  package.json, and detect-only test)
- Remove Enumerable import and extension from EmberArray
- Remove MutableEnumerable import and extension from MutableArray
- Remove MutableEnumerable re-export from @ember/-internals/runtime
- Clean up Enumerable entries in root package.json, lib/index.js,
  tree-shakability test, and workspace package.json dependencies
- Delete mutable_enumerable_test.js, which only tested MutableEnumerable.detect

BREAKING CHANGE: @ember/enumerable and @ember/enumerable/mutable are removed.
Legacy Enumerable.detect and MutableEnumerable.detect checks are no longer
supported. Code that needs to detect Ember array behavior should use
EmberArray.detect or MutableArray.detect where appropriate.
- Delete @ember/object/promise-proxy-mixin implementation
- Remove promise-proxy-mixin package exports and entrypoints
- Delete PromiseProxyMixin-specific runtime tests
- Remove tracked-test case that depended on ArrayProxy.extend(PromiseProxyMixin)
- Clean up docs expectations and tree-shakability entries

BREAKING CHANGE: @ember/object/promise-proxy-mixin is removed.
Code that used ObjectProxy.extend(PromiseProxyMixin) must migrate away from
PromiseProxyMixin and manage promise state explicitly.
- Move TargetActionSupport#triggerAction directly onto Component
- Move ActionSupport#send directly onto Component
- Preserve triggerAction target/action/actionContext behavior and typings
- Replace TargetActionSupport mixin tests with Component#triggerAction coverage
- Remove ActionSupport and TargetActionSupport exports and entrypoints
- Clean up tree-shakability expectations and stale mixin comments

BREAKING CHANGE: private ActionSupport and TargetActionSupport mixin modules are removed.
Component still provides send() and triggerAction() directly.
Inline proxy behavior directly into ObjectProxy.

Move shared proxy tracking helpers into runtime/lib/proxy-utils:
- ContentProxy
- contentFor
- customTagForProxy

Remove the private _ProxyMixin runtime export and delete the old
runtime/lib/mixins/-proxy module. Update internal imports, package
entrypoints, and tree-shakability expectations accordingly.

BREAKING CHANGE: private _ProxyMixin and runtime/lib/mixins/-proxy are removed.
ObjectProxy still provides content forwarding, unknownProperty,
setUnknownProperty, and isTruthy directly.
- Inline RegistryProxyMixin behavior directly into Engine and EngineInstance
- Inline ContainerProxyMixin behavior directly into EngineInstance
- Remove RegistryProxyMixin and ContainerProxyMixin runtime exports
- Delete the old runtime/lib/mixins/registry_proxy and container_proxy modules
- Update package entrypoints and tree-shakability expectations
- Move container proxy behavior coverage from the deleted mixin test to EngineInstance

BREAKING CHANGE: private RegistryProxyMixin and ContainerProxyMixin mixin modules are removed.
Engine and EngineInstance still provide the same registry/container proxy APIs directly.
Copy link
Copy Markdown
Contributor

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

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

thanks a ton for working on this! I see some things that can be extracted to separate PRs, and some things need to come back, but have deprecations added

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.

we can't delete stuff yet -- it has to stick around until v8

*/
without(value: T): NativeArray<T>;
}
const EmberArray = Mixin.create(Enumerable, {
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.

EmberArray is already deprecated (via RFC):

but we haven't shipped the EmberArray deprecation quite yet

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.

so this means we can probably add deprecations to the whole thing, rather than deprecation the Enumerable methods on here

interface Engine extends RegistryProxyMixin {}
class Engine extends Namespace.extend(RegistryProxyMixin) {
interface Engine extends RegistryProxy {}
class Engine extends Namespace {
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.

these are good changes -- whatever is in RegistryProxyMixin should probably be inlined in here (so as to not use a mixin) -- exactly what you've done 🎉

class EngineInstance extends EmberObject.extend(RegistryProxyMixin, ContainerProxyMixin) {
// `EngineInstance` implements the registry/container proxy APIs directly.
interface EngineInstance extends InternalOwner, Owner {}
class EngineInstance extends EmberObject {
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.

these are good changes, and we could probably ship them in separate PRs -- any change that keeps the API the same, but removes a mixin is a good candidate for separate PR

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.

can't remove this quite yet -- we could in v8 -- but we need to ship the deprecation first

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.

2 participants