Repository navigation
RFC#1234 & RFC#1247: Spike: every classic feature behind EMBER_DROP_CLASSIC_FEATURES - #21773
Draft
NullVoxPopuli-ai-agent wants to merge 27 commits into
Draft
NullVoxPopuli-ai-agent wants to merge 27 commits into
NullVoxPopuli-ai-agent wants to merge 27 commits into
Conversation
`import.meta.env?.EMBER_DROP_CLASSIC_FEATURES` is a build-time flag for RFC 1247. Later changes read it where a classic feature is defined, and remove that feature from builds that set the flag. - `ImportMetaEnv` declares the flag for the type checks. - `EMBER_DROP_` is in the Vite `envPrefix`, so `EMBER_DROP_CLASSIC_FEATURES=true pnpm vite build` sets it for the tests. - CI has a "Classic features dropped" variant that builds and tests with the flag. - The hello-world smoke test sets the flag with a Vite `define` entry, so the package-size workflow measures an app without the classic features. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the flag, `@ember/object/computed` exports `undefined` for the 15 macros of the `deprecate-computed-macros` deprecation: `alias`, `and`, `bool`, `deprecatingAlias`, `equal`, `gt`, `gte`, `lt`, `lte`, `match`, `not`, `oneWay`, `or`, `readOnly` and `reads`. A cast keeps the published types. Tests of these macros skip with the flag, as they skip when the deprecation is removed. A new test checks that the exports are `undefined` in a build with the flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the flag, `@ember/object/evented`, `@ember/object/observable`, `@ember/object/promise-proxy-mixin` and the `ControllerMixin` export of `@ember/controller` export `undefined`. A cast keeps the published types, and a type alias keeps the type half of each export. `Route`, `EmberRouter` and `CoreView` add `Evented` to their meta only so that `Evented.detect` stays true, so they skip that with the flag. The tests of these mixins skip with the flag. One test in `tracked-test.js` used `PromiseProxyMixin` without a skip for its removal, and has one now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the flag, `@ember/service` exports a `Service` class that extends no other class. The owner passes itself to the constructor, as it does for Glimmer components. A static `create` is the only entry point that the container needs. Without the flag, `Service` stays the same, and a cast keeps the published types. `willDestroy` stays the teardown hook. The plain class has no `destroy` method. The container calls `willDestroy`, then the `destroy` function of `@ember/destroyable`, for classes marked with `setPlainFrameworkClass`. Other instances keep the `destroy()` call. The internal router and routing services extend the exported class, so they run on the plain class in the flag build. Tests: - Three tests that used `init()` on a service now use a constructor. - The router service tests read properties without `get()`. - `set(service, ...)` replaces `service.set(...)` in one test. - The `ProxyMixin` service test skips with the flag. - A flag-only module checks the plain class. - `packages/ember/tests/without-EMBER_DROP_CLASSIC_FEATURES/service_test.js` checks the classic API in builds without the flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the flag, `Helper` from `@ember/component/helper` is a class that extends no other class. The owner passes itself to the constructor. `invokeHelper` can create a helper without an owner, so the owner is optional. `compute` and `recompute` work as before. A static `create` stays, because the container and the helper manager create instances through it. `willDestroy` stays the teardown hook. The base has a `destroy()` method that calls the `destroy` function of `@glimmer/destroyable`, and an empty `willDestroy()`, so `super.willDestroy(...arguments)` works. Only a subclass that overrides `willDestroy` gets a destructor for it. Without the flag, `Helper` stays the same. A cast keeps the published types. Tests: - With the flag, `registerHelper` puts an object body on the prototype of a `Helper` subclass, because `Helper.extend` does not exist. - One test used `init()` on a helper and now uses a constructor. One test read an injection with `get()` and now reads the property. - 10 tests in `custom-helper-test.js` and 2 in `unbound-test.js` use `init`, `_super` or observers, and skip with the flag. - A new flag-only module checks the plain class. - `packages/ember/tests/without-EMBER_DROP_CLASSIC_FEATURES/helper_test.js` checks the classic API in builds without the flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the flag, `Route` from `@ember/routing/route` extends a private plain base class, not `EmberObject` and the `ActionHandler` mixin. The class body is the same in both modes. The base sets the owner in the constructor and keeps a static `create` for the container. A route without an owner is allowed, as before. `willDestroy` stays the teardown hook. The base has a `destroy()` method that calls the `destroy` function of `@glimmer/destroyable`, and an empty `willDestroy()`. Only a subclass that overrides `willDestroy` gets a destructor for it. `Route` overrides it to call `teardownViews`, so that runs with the flag too. The defaults that `Route.reopen` set (`queryParams`, `templateName`, `controllerName`, `send` and `actions`) go on the prototype with the flag. Without the flag, `reopen` still sets them and merges `queryParams`. `send` reads the destroy state through `@glimmer/destroyable`, so it works on both bases. A cast keeps the published types. Tests: - Route subclasses in tests use a constructor, not `init()`, and `super`, not `this._super`. - The route unit tests set properties after `create()`, read with `get()`, and destroy with the `destroy` function. - The test of the `queryParams` array assertion skips with the flag. The assertion comes from the merge of `Route.extend()`. - A new flag-only module checks the plain base. - A new suite in `packages/ember/tests/without-EMBER_DROP_CLASSIC_FEATURES` checks the classic API in builds without the flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the flag, `@ember/controller` exports a `Controller` class that extends no other class. The owner passes itself to the constructor. The class has a tracked `model`, `queryParams`, `target`, `namespace`, `isController`, and the `_qpDelegate`/`_qpChanged` pair that the route uses for query params. A static `create` is the entry point for the container. The base has `destroy()`, which calls `destroy` from `@glimmer/destroyable`, and an empty `willDestroy()`. If a subclass overrides `willDestroy`, it runs as a destructor. Without the flag, `Controller` stays the same. A cast keeps the published types. The mount manager sets the model with `set()`, because the plain class has no `set` method and ignores the properties that `create` receives. Tests: - Tests that call `get`, `set`, `setProperties`, `getProperties` or `toggleProperty` on a controller now use the functions. - Tests that used `init()` on a controller now use a constructor. - The query param test helpers define native classes. - Tests of `send`, `actions`, concatenated or computed `queryParams` and `addObserver` on a controller skip with the flag. - A new flag-only module checks the plain class. - `packages/ember/tests/without-EMBER_DROP_CLASSIC_FEATURES/controller_test.js` checks the classic API in builds without the flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The default export of `@ember/controller` was a cast `const`. Apps that emit
declarations then got TS4023 for `export const X = Controller.extend({})`,
because the instance type could not be named.
`Controller` is now a class declaration again. It extends `ControllerBase`,
which is the plain base class with EMBER_DROP_CLASSIC_FEATURES and the
classic base without it. A type test covers the declaration emit.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…EATURES With the flag, the default exports of `@ember/object` and `@ember/object/core` are `undefined`. A cast keeps the published types. The classes move to private modules, so framework code keeps them in the flag build: - `@ember/object/core.ts` moves to `@ember/object/lib/core-object.ts`. - The `EmberObject` class moves to `@ember/object/lib/ember-object.ts`. - Both files are in the rollup ignore list, so they are not module paths of the published package. - Framework code imports the classes from these files. Tests: - Test files import the classes from the private modules, so the same tests run in every build. - A flag-only test checks that the two exports are `undefined`. - `packages/ember/tests/without-EMBER_DROP_CLASSIC_FEATURES/ember_object_test.js` checks the public classes in builds without the flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the flag, the rest of the computed property API exports `undefined`:
- `computed` from `@ember/object`
- `dependentKeyCompat` from `@ember/object/compat`
- from `@ember/object/computed`: the default export (`ComputedProperty`),
`expandProperties`, `empty`, `notEmpty`, `none`, and the array macros
`sum`, `max`, `min`, `map`, `mapBy`, `filter`, `filterBy`, `uniq`,
`uniqBy`, `union`, `intersect`, `setDiff`, `collect` and `sort`
A cast keeps the published types, and a type alias keeps the type half of
the default export.
Ember code keeps the implementation. The code of `dependentKeyCompat` moves
to the private `@ember/object/lib/dependent-key-compat.ts`, which `Route`
and `RouterService` import. Rollup bundles that file into a shared chunk,
so it does not become a module path.
Tests:
- Tests that use these APIs skip with the flag. The array macro tests and
one `{{#each}}` module with a computed `arrangedContent` register only
without the flag.
- The shared array test helpers use the internal `computed`, because the
flag test page must load.
- The flag-only module checks that the exports are `undefined`.
- `packages/ember/tests/without-EMBER_DROP_CLASSIC_FEATURES/computed_test.js`
checks the API in builds without the flag.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # packages/@ember/-internals/glimmer/tests/integration/custom-component-manager-test.js
…p-classic-all # Conflicts: # packages/ember/tests/routing/query_params_test.js
…op-classic-all # Conflicts: # packages/@ember/-internals/runtime/tests/core/is_array_test.js # packages/@ember/-internals/runtime/tests/helpers/array.js # packages/@ember/-internals/runtime/tests/mixins/observable_test.js # packages/@ember/object/core.ts # packages/@ember/object/tests/extend_test.js # packages/@ember/utils/tests/type_of_test.js # packages/ember/tests/routing/model_loading_test.js # packages/ember/tests/routing/query_params_test.js # packages/ember/tests/routing/template_rendering_test.js # rollup.config.mjs
@ember/test-helpers 5.7.0 extends EmberObject at module scope, so its module does not load when EMBER_DROP_CLASSIC_FEATURES removes the EmberObject export. ember.js calls setApplication(), so the tests never build the mock owner. emberjs/ember-test-helpers#1584 removes the class. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the flag, the public EmberObject export is undefined, so `instanceof EmberObject` throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the flag, these public exports are `undefined`: - `Component` from `@ember/component` - `Mixin` from `@ember/object/mixin` - `EmberArray`, `A`, `MutableArray` and `NativeArray` from `@ember/array`, and `@ember/array/mutable` - `ArrayProxy` and `ObjectProxy` - `Enumerable` and `MutableEnumerable` - `observer` from `@ember/object`, and `addObserver` and `removeObserver` The implementations move to private modules in the rollup ignore list, so framework code keeps them. Test files import the private modules, so the same tests run in every build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the flag, these classes do not extend EmberObject: - Namespace, Engine, EngineInstance, Application, ApplicationInstance - EmberRouter, HashLocation, HistoryLocation, NoneLocation - EventDispatcher, DataAdapter, ContainerDebugAdapter Their base is the new private PlainFrameworkObject. Its `create` assigns the properties, then calls `init`, in the same order as EmberObject, so the class bodies work on both bases. Engine and EngineInstance get the registry and container proxy methods as class bodies in place of the mixins. The classic bases are built only in builds without the flag, so the flag build does not keep EmberObject alive through them. Route and Controller now build their classic base the same way. The container marks the plain Service with a static field in place of a call at module scope, so @ember/service has no side effects. Test code that called classic methods on routers, locations and applications now uses the get and set functions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A top-level `if` keeps the module in the tree-shakability snapshot. In a static block, the defaults go away with the class when nothing imports it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
With EMBER_DROP_CLASSIC_FEATURES:
- `get` and template property reads do not call `unknownProperty`.
- `set` does not call `setUnknownProperty`.
- `{{each-in}}` does not unwrap proxies, and conditionals do not read
`isTruthy` from a proxy.
So an app that only renders components loads no proxy or mixin code. The
minified hello-world smoke app goes from 129,289 to 116,820 bytes.
The tests of proxy behavior skip with the flag.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With EMBER_DROP_CLASSIC_FEATURES, these exports are plain classes that extend no other class. The classic classes do not change. - Application, ApplicationInstance, Engine, EngineInstance: separate plain implementations. Engine builds its registry on first use, so it sees the Resolver field of subclasses. Application.create(options) assigns the options, then starts the application. These keep a public destroy() method. - HashLocation, HistoryLocation, NoneLocation: separate plain implementations with their setup in the constructor. - EmberRouter, EventDispatcher, DataAdapter, ContainerDebugAdapter: these already set themselves up in the constructor, so they get a small plain base. The container calls willDestroy, then destroys them with @ember/destroyable, as for Service. - Namespace: the export is undefined. The class moves to a private module, because the classic Engine extends it. The classic bases of Engine and EngineInstance are built in a function, so the flag build does not create them when the module loads. The exported types are interfaces, so that declaration files can name them. Tests: - Engine and Application tests register their factories in an initializer, not in init(), so they work on both classes. - Location unit tests of the classic classes run only without the flag. A flag-only module tests the plain location classes. - Other tests that passed properties to create() use subclasses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Spike: every classic feature behind
EMBER_DROP_CLASSIC_FEATURESin one branch. Do not merge this PR. It shows the end state of the stack and what is still left.This branch merges all the PRs of the stack onto
main. It adds the last group of exports, and puts the core classes on a plain base:deprecate-computed-macroscomputed,dependentKeyCompatand the other computed exportsEvented,Observable,PromiseProxyMixin,ControllerMixinServiceas a plain classHelperas a plain classRouteon a plain base classControlleras a plain classEmberObjectandCoreObjectNew in this PR
With the flag, these exports are
undefined:Componentfrom@ember/componentMixinfrom@ember/object/mixinEmberArray,A,MutableArray,NativeArrayfrom@ember/array, and@ember/array/mutableArrayProxy,ObjectProxy,Enumerable,MutableEnumerableobserverfrom@ember/object, andaddObserverandremoveObserverfrom@ember/object/observersEach implementation moves to a private module in the rollup ignore list, as in #21743. Framework code and test files import the private module, so the same tests run in every build.
New tests:
packages/ember/tests/drop_classic_features_exports_test.jschecks that the exports areundefined. It runs only with the flag.packages/ember/tests/without-EMBER_DROP_CLASSIC_FEATURES/classic_exports_test.jschecks that each public export is the private implementation. It runs only without the flag.Core classes
With the flag, the exports below are plain classes that extend no other class. The classic classes do not change.
Application,ApplicationInstance,Engine,EngineInstancedestroy()HashLocation,HistoryLocation,NoneLocationEmberRouter,EventDispatcher,DataAdapter,ContainerDebugAdapterNamespaceundefinedEnginebuilds its registry on first use, so it sees theResolverfield of subclasses.Application.create(options)assigns the options, then starts the application. The constructor cannot start it, because the fields of subclasses do not exist yet when it runs.willDestroy, then destroys the router, the locations and the event dispatcher with@ember/destroyable, as forService.Tests:
init(), so they work on both classes.packages/@ember/routing/tests/location/drop_classic_features_test.jstests the plain ones.packages/ember/tests/drop_classic_features_core_classes_test.jschecks that no core class extendsCoreObject, and checksApplication.create, the engine registry andwillDestroy.Rendering without proxies
With the flag:
getand template property reads do not callunknownProperty.setdoes not callsetUnknownProperty.{{each-in}}does not unwrap proxies, and conditionals do not readisTruthyfrom a proxy.The tests of proxy behavior skip with the flag.
Other fixes
@ember/servicehad a side effect when it loaded. A static field now marks the plain class, and the tree-shakability snapshot is the same as onmain.@ember/test-helpers5.7.0 extendsEmberObjectat module scope. Apnpm patchcreates that class on first use. Delete the patch when Migrate the test-specific Owner to not rely on imminently deprecated features (mixins and EmberObject) ember-test-helpers#1584 ships.Bundle size
smoke-tests/v2-app-hello-world-templaterenders one component. Its config sets the flag.mainThe hello-world bundle has no
CoreObject,EmberObject, mixin, proxy orNamespacecode.It still has the observer code. Query params sync the controller to the URL through internal observers, also for
@trackedproperties. Removing observers needs a new query param sync first.smoke-tests/v2-app-templateusesclassicEmberSupport()and classic dependencies, so its bundle keeps the classic code with the flag.Checks
On
mainat 6772f7b, at 465bd0d.ALL_DEPRECATIONS_ENABLED=trueOVERRIDE_DEPRECATION_VERSION=15.0.0EMBER_DROP_CLASSIC_FEATURES=trueVITE_STABLE_DECORATORS=trueVITE_STABLE_DECORATORS=typescriptThe type checks of the internals and of the published types (with and without stable decorators),
eslint,prettier, the docs check and the tree-shakability test pass.Supersedes the old spike #21674.
🤖 Generated with Claude Code