From 029cc2e364f91f21b763b965e11d6ace17c1cac6 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 6 Mar 2025 10:00:13 -0500 Subject: [PATCH 01/23] setting up another stable decorators experiment --- babel.config.mjs | 15 ++-- package.json | 5 +- .../-internals/metal/lib/decorator-util.ts | 86 +++++++++++++++++++ pnpm-lock.yaml | 5 +- rollup.config.mjs | 8 +- tsconfig/compiler-options.json | 1 - vite.config.mjs | 4 + 7 files changed, 109 insertions(+), 15 deletions(-) create mode 100644 packages/@ember/-internals/metal/lib/decorator-util.ts diff --git a/babel.config.mjs b/babel.config.mjs index 34b6aa42dee..529289ed189 100644 --- a/babel.config.mjs +++ b/babel.config.mjs @@ -17,13 +17,14 @@ export default { allowDeclareFields: true, }, ], - [ - 'module:decorator-transforms', - { - runEarly: true, - runtime: { import: 'decorator-transforms/runtime' }, - }, - ], + ['@babel/plugin-proposal-decorators', { version: '2023-11' }], + // [ + // 'module:decorator-transforms', + // { + // runEarly: true, + // runtime: { import: 'decorator-transforms/runtime' }, + // }, + // ], [ 'babel-plugin-ember-template-compilation', { diff --git a/package.json b/package.json index a1b7598b43d..27947981762 100644 --- a/package.json +++ b/package.json @@ -107,7 +107,8 @@ }, "devDependencies": { "@aws-sdk/client-s3": "^3.731.0", - "@babel/plugin-transform-typescript": "^7.22.9", + "@babel/plugin-proposal-decorators": "^7.25.9", + "@babel/plugin-transform-typescript": "^7.26.8", "@babel/preset-env": "^7.16.11", "@babel/types": "^7.22.5", "@embroider/shared-internals": "^2.5.0", @@ -400,4 +401,4 @@ } }, "packageManager": "pnpm@10.5.0" -} \ No newline at end of file +} diff --git a/packages/@ember/-internals/metal/lib/decorator-util.ts b/packages/@ember/-internals/metal/lib/decorator-util.ts new file mode 100644 index 00000000000..68815852676 --- /dev/null +++ b/packages/@ember/-internals/metal/lib/decorator-util.ts @@ -0,0 +1,86 @@ +/* + Types and utilities for working with 2023-11 decorators -- the ones that are + currently (as of 2025-05-05) in Stage 3. + + TypeScript provides built-in types for all the Context objects, but not a way + to do type discrimination against the `value` argument. +*/ + +export type ClassMethodDecorator = ( + value: Function, + context: ClassMethodDecoratorContext +) => Function | void; + +export type ClassGetterDecorator = ( + value: Function, + context: ClassGetterDecoratorContext +) => Function | void; + +export type ClassSetterDecorator = ( + value: Function, + context: ClassSetterDecoratorContext +) => Function | void; + +export type ClassFieldDecorator = ( + value: undefined, + context: ClassFieldDecoratorContext +) => (initialValue: unknown) => unknown | void; + +export type ClassDecorator = (value: Function, context: ClassDecoratorContext) => Function | void; + +export type ClassAutoAccessorDecorator = ( + value: ClassAccessorDecoratorTarget, + context: ClassAccessorDecoratorContext +) => ClassAccessorDecoratorResult; + +export type Decorator = + | ClassMethodDecorator + | ClassGetterDecorator + | ClassSetterDecorator + | ClassFieldDecorator + | ClassDecorator + | ClassAutoAccessorDecorator; + +export function isModernDecoratorArgs(args: unknown[]): args is Parameters { + return args.length === 2 && typeof args[1] === 'object' && args[1] != null && 'kind' in args[1]; +} + +// this is designed to turn the arguments into a discriminated union so you can +// check the kind once and then have the right types for them. +export function identifyModernDecoratorArgs(args: Parameters): + | { + kind: 'method'; + value: Parameters[0]; + context: Parameters[1]; + } + | { + kind: 'getter'; + value: Parameters[0]; + context: Parameters[1]; + } + | { + kind: 'setter'; + value: Parameters[0]; + context: Parameters[1]; + } + | { + kind: 'field'; + value: Parameters[0]; + context: Parameters[1]; + } + | { + kind: 'class'; + value: Parameters[0]; + context: Parameters[1]; + } + | { + kind: 'accessor'; + value: Parameters[0]; + context: Parameters[1]; + } { + return { + kind: args[1].kind, + value: args[0], + context: args[1], + } as ReturnType; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6bb1cba6b91..43907ed5be0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -139,8 +139,11 @@ importers: '@aws-sdk/client-s3': specifier: ^3.731.0 version: 3.750.0 + '@babel/plugin-proposal-decorators': + specifier: ^7.25.9 + version: 7.25.9(@babel/core@7.26.9) '@babel/plugin-transform-typescript': - specifier: ^7.22.9 + specifier: ^7.26.8 version: 7.26.8(@babel/core@7.26.9) '@babel/preset-env': specifier: ^7.16.11 diff --git a/rollup.config.mjs b/rollup.config.mjs index cebcfd49e72..60b8c3101f7 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -247,10 +247,10 @@ export function hiddenDependencies() { 'module' ).path, ...walkGlimmerDeps(['@glimmer/compiler']), - 'decorator-transforms/runtime': resolve( - findFromProject('decorator-transforms').root, - 'dist/runtime.js' - ), + // 'decorator-transforms/runtime': resolve( + // findFromProject('decorator-transforms').root, + // 'dist/runtime.js' + // ), }; } diff --git a/tsconfig/compiler-options.json b/tsconfig/compiler-options.json index 91afd83639d..2f862e5c0f8 100644 --- a/tsconfig/compiler-options.json +++ b/tsconfig/compiler-options.json @@ -9,7 +9,6 @@ "rootDir": "../packages", // Environment Configuration - "experimentalDecorators": true, "moduleResolution": "bundler", // Enhance Strictness diff --git a/vite.config.mjs b/vite.config.mjs index 87c4ac14afd..b39ec43d6be 100644 --- a/vite.config.mjs +++ b/vite.config.mjs @@ -43,6 +43,10 @@ export default defineConfig(({ mode }) => { optimizeDeps: { noDiscovery: true }, publicDir: 'tests/public', build, + + // the stock esbuild support for typescript is horribly broken. For example, + // it will simply remove your decorators. + esbuild: false, }; }); From 2bf57d1f7bfec9cf5c53c4b116ca0f90a2241930 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 6 Mar 2025 11:10:17 -0500 Subject: [PATCH 02/23] tracked tests passing --- babel.config.mjs | 19 +++++---- .../@ember/-internals/metal/lib/computed.ts | 6 +++ .../@ember/-internals/metal/lib/decorator.ts | 39 +++++++++++++++---- .../@ember/-internals/metal/lib/tracked.ts | 28 +++++++++++++ .../metal/tests/tracked/validation_test.js | 8 +++- rollup.config.mjs | 12 ++++-- 6 files changed, 91 insertions(+), 21 deletions(-) diff --git a/babel.config.mjs b/babel.config.mjs index 529289ed189..bca99f4cec6 100644 --- a/babel.config.mjs +++ b/babel.config.mjs @@ -17,14 +17,17 @@ export default { allowDeclareFields: true, }, ], - ['@babel/plugin-proposal-decorators', { version: '2023-11' }], - // [ - // 'module:decorator-transforms', - // { - // runEarly: true, - // runtime: { import: 'decorator-transforms/runtime' }, - // }, - // ], + ...(process.env.VITE_STABLE_DECORATORS + ? [['@babel/plugin-proposal-decorators', { version: '2023-11' }]] + : [ + [ + 'module:decorator-transforms', + { + runEarly: true, + runtime: { import: 'decorator-transforms/runtime' }, + }, + ], + ]), [ 'babel-plugin-ember-template-compilation', { diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts index aac84147b84..93e74c0cd97 100644 --- a/packages/@ember/-internals/metal/lib/computed.ts +++ b/packages/@ember/-internals/metal/lib/computed.ts @@ -40,6 +40,7 @@ import { notifyPropertyChange, PROPERTY_DID_CHANGE, } from './property_events'; +import { isModernDecoratorArgs } from './decorator-util'; export type ComputedPropertyGetterFunction = (this: any, key: string) => unknown; export type ComputedPropertySetterFunction = ( @@ -884,6 +885,11 @@ export function computed(callback: ComputedPropertyCallback): ComputedDecorator; export function computed( ...args: ElementDescriptor | string[] | ComputedDecoratorKeysAndConfig ): ComputedDecorator | DecoratorPropertyDescriptor | void { + if (isModernDecoratorArgs(args)) { + console.log(`ignoring computed on ${args[1].kind} ${args[1].name?.toString()}`); + return; + } + assert( `@computed can only be used directly as a native decorator. If you're using tracked in classic classes, add parenthesis to call it like a function: computed()`, !(isElementDescriptor(args.slice(0, 3)) && args.length === 5 && (args[4] as unknown) === true) diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index d2bf0ff2ccb..3100bf9ba0a 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -2,6 +2,11 @@ import type { Meta } from '@ember/-internals/meta'; import { meta as metaFor, peekMeta } from '@ember/-internals/meta'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; +import { + type Decorator, + identifyModernDecoratorArgs, + isModernDecoratorArgs, +} from './decorator-util'; export type DecoratorPropertyDescriptor = (PropertyDescriptor & { initializer?: any }) | undefined; @@ -113,13 +118,23 @@ export function makeComputedDecorator( desc: ComputedDescriptor, DecoratorClass: { prototype: object } ): ExtendedMethodDecorator { - let decorator = function COMPUTED_DECORATOR( - target: object, - key: string, - propertyDesc?: DecoratorPropertyDescriptor, - maybeMeta?: Meta, - isClassicDecorator?: boolean - ): DecoratorPropertyDescriptor { + let decorator = function COMPUTED_DECORATOR(...args: unknown[]): DecoratorPropertyDescriptor { + if (isModernDecoratorArgs(args)) { + return computedDecorator2023( + args, + desc, + DecoratorClass + ) as unknown as DecoratorPropertyDescriptor; + } + + let [target, key, propertyDesc, maybeMeta, isClassicDecorator] = args as [ + object, + string, + DecoratorPropertyDescriptor | undefined, + Meta | undefined, + boolean | undefined + ]; + assert( `Only one computed property decorator can be applied to a class field or accessor, but '${key}' was decorated twice. You may have added the decorator to both a getter and setter, which is unnecessary.`, isClassicDecorator || @@ -148,6 +163,16 @@ export function makeComputedDecorator( return decorator; } +function computedDecorator2023( + args: Paramters, + desc: ComputedDescriptor, + DecoratorClass: { prototype: object } +) { + let dec = identifyModernDecoratorArgs(args); + console.log(`ignoring modern decorator on ${dec.kind} ${dec.context.name?.toString()}`); + return; +} + ///////////// const DECORATOR_DESCRIPTOR_MAP: WeakMap = diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index f5f696997f5..0722e74ae7b 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -8,6 +8,7 @@ import { CHAIN_PASS_THROUGH } from './chain-tags'; import type { ExtendedMethodDecorator, DecoratorPropertyDescriptor } from './decorator'; import { COMPUTED_SETTERS, isElementDescriptor, setClassicDecorator } from './decorator'; import { SELF_TAG } from './tags'; +import { Decorator, identifyModernDecoratorArgs, isModernDecoratorArgs } from './decorator-util'; /** @decorator @@ -82,6 +83,11 @@ export function tracked( desc: DecoratorPropertyDescriptor ): DecoratorPropertyDescriptor; export function tracked(...args: any[]): ExtendedMethodDecorator | DecoratorPropertyDescriptor { + if (isModernDecoratorArgs(args)) { + // TODO: cast is a lie, keeping the public types unchanged for now + return tracked2023(args) as unknown as DecoratorPropertyDescriptor; + } + assert( `@tracked can only be used directly as a native decorator. If you're using tracked in classic classes, add parenthesis to call it like a function: tracked()`, !(isElementDescriptor(args.slice(0, 3)) && args.length === 5 && args[4] === true) @@ -200,3 +206,25 @@ export class TrackedDescriptor { this._set.call(obj, value); } } + +function tracked2023(args: Parameters) { + const dec = identifyModernDecoratorArgs(args); + switch (dec.kind) { + case 'field': + dec.context.addInitializer(function (this: any) { + let initial = this[dec.context.name]; + Object.defineProperty( + this, + dec.context.name, + descriptorForField([ + this, + dec.context.name as string, + { initializer: () => initial }, + ]) as any + ); + }); + return; + default: + throw new Error(`unimplemented: tracked on ${dec.kind} ${dec.context.name?.toString()}`); + } +} diff --git a/packages/@ember/-internals/metal/tests/tracked/validation_test.js b/packages/@ember/-internals/metal/tests/tracked/validation_test.js index 307c35dcc4e..f2d466238ab 100644 --- a/packages/@ember/-internals/metal/tests/tracked/validation_test.js +++ b/packages/@ember/-internals/metal/tests/tracked/validation_test.js @@ -54,7 +54,11 @@ moduleFor( let tag = track(() => obj.first); let snapshot = valueForTag(tag); - assert.equal(obj.first, 'first: second', 'The value initializes correctly'); + let expectedInitialValue = import.meta.env.VITE_STABLE_DECORATORS + ? 'first: undefined' + : 'first: second'; + + assert.equal(obj.first, expectedInitialValue, 'The value initializes correctly'); assert.equal(validateTag(tag, snapshot), true); snapshot = valueForTag(tag); @@ -65,7 +69,7 @@ moduleFor( // See: https://github.com/glimmerjs/glimmer-vm/pull/1018 // assert.equal(validate(tag, snapshot), true); - assert.equal(obj.first, 'first: second', 'The value stays the same once initialized'); + assert.equal(obj.first, expectedInitialValue, 'The value stays the same once initialized'); snapshot = valueForTag(tag); assert.equal(validateTag(tag, snapshot), true); diff --git a/rollup.config.mjs b/rollup.config.mjs index 60b8c3101f7..e89f7ab8cb3 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -247,10 +247,14 @@ export function hiddenDependencies() { 'module' ).path, ...walkGlimmerDeps(['@glimmer/compiler']), - // 'decorator-transforms/runtime': resolve( - // findFromProject('decorator-transforms').root, - // 'dist/runtime.js' - // ), + ...(process.env.VITE_STABLE_DECORATORS + ? {} + : { + 'decorator-transforms/runtime': resolve( + findFromProject('decorator-transforms').root, + 'dist/runtime.js' + ), + }), }; } From 3468f2109787d8e4f2ca3a42322859ce4ac35220 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 6 Mar 2025 15:02:38 -0500 Subject: [PATCH 03/23] more passing tests --- .../@ember/-internals/metal/lib/decorator.ts | 75 +++++++++++-------- .../-internals/metal/lib/injected_property.ts | 45 +++++++++++ packages/@ember/object/index.ts | 26 +++++++ packages/@ember/service/tests/service_test.js | 1 - 4 files changed, 115 insertions(+), 32 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index 3100bf9ba0a..5600f552f4b 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -120,11 +120,7 @@ export function makeComputedDecorator( ): ExtendedMethodDecorator { let decorator = function COMPUTED_DECORATOR(...args: unknown[]): DecoratorPropertyDescriptor { if (isModernDecoratorArgs(args)) { - return computedDecorator2023( - args, - desc, - DecoratorClass - ) as unknown as DecoratorPropertyDescriptor; + return computedDecorator2023(args, desc) as unknown as DecoratorPropertyDescriptor; } let [target, key, propertyDesc, maybeMeta, isClassicDecorator] = args as [ @@ -135,25 +131,7 @@ export function makeComputedDecorator( boolean | undefined ]; - assert( - `Only one computed property decorator can be applied to a class field or accessor, but '${key}' was decorated twice. You may have added the decorator to both a getter and setter, which is unnecessary.`, - isClassicDecorator || - !propertyDesc || - !propertyDesc.get || - !COMPUTED_GETTERS.has(propertyDesc.get) - ); - - let meta = arguments.length === 3 ? metaFor(target) : maybeMeta; - desc.setup(target, key, propertyDesc, meta!); - - let computedDesc: PropertyDescriptor = { - enumerable: desc.enumerable, - configurable: desc.configurable, - get: DESCRIPTOR_GETTER_FUNCTION(key, desc), - set: DESCRIPTOR_SETTER_FUNCTION(key, desc), - }; - - return computedDesc; + return makeDescriptor(desc, target, key, propertyDesc, maybeMeta, isClassicDecorator); }; setClassicDecorator(decorator, desc); @@ -163,14 +141,49 @@ export function makeComputedDecorator( return decorator; } -function computedDecorator2023( - args: Paramters, +function makeDescriptor( desc: ComputedDescriptor, - DecoratorClass: { prototype: object } -) { - let dec = identifyModernDecoratorArgs(args); - console.log(`ignoring modern decorator on ${dec.kind} ${dec.context.name?.toString()}`); - return; + target: object, + key: string, + propertyDesc?: DecoratorPropertyDescriptor, + maybeMeta?: Meta, + isClassicDecorator?: boolean +): PropertyDescriptor { + assert( + `Only one computed property decorator can be applied to a class field or accessor, but '${key}' was decorated twice. You may have added the decorator to both a getter and setter, which is unnecessary.`, + isClassicDecorator || + !propertyDesc || + !propertyDesc.get || + !COMPUTED_GETTERS.has(propertyDesc.get) + ); + + let meta = arguments.length === 3 ? metaFor(target) : maybeMeta; + desc.setup(target, key, propertyDesc, meta!); + + let computedDesc: PropertyDescriptor = { + enumerable: desc.enumerable, + configurable: desc.configurable, + get: DESCRIPTOR_GETTER_FUNCTION(key, desc), + set: DESCRIPTOR_SETTER_FUNCTION(key, desc), + }; + return computedDesc; +} + +function computedDecorator2023(args: Parameters, desc: ComputedDescriptor) { + const dec = identifyModernDecoratorArgs(args); + switch (dec.kind) { + case 'field': + dec.context.addInitializer(function (this: any) { + Object.defineProperty( + this, + dec.context.name, + makeDescriptor(desc, this, dec.context.name as string) + ); + }); + return; + default: + console.log(`unimplemented: injected on ${dec.kind} ${dec.context.name?.toString()}`); + } } ///////////// diff --git a/packages/@ember/-internals/metal/lib/injected_property.ts b/packages/@ember/-internals/metal/lib/injected_property.ts index 1228bf97454..a07d76c00f6 100644 --- a/packages/@ember/-internals/metal/lib/injected_property.ts +++ b/packages/@ember/-internals/metal/lib/injected_property.ts @@ -5,6 +5,11 @@ import { computed } from './computed'; import type { DecoratorPropertyDescriptor, ElementDescriptor } from './decorator'; import { isElementDescriptor } from './decorator'; import { defineProperty } from './properties'; +import { + type Decorator, + identifyModernDecoratorArgs, + isModernDecoratorArgs, +} from './decorator-util'; export let DEBUG_INJECTION_FUNCTIONS: WeakMap; @@ -49,6 +54,10 @@ function inject( let elementDescriptor; let name: string | undefined; + if (isModernDecoratorArgs(args)) { + return inject2023(type, undefined, args); + } + if (isElementDescriptor(args)) { elementDescriptor = args; } else if (typeof args[0] === 'string') { @@ -88,4 +97,40 @@ function inject( } } +function inject2023(type: string, name: string | undefined, args: Parameters) { + const dec = identifyModernDecoratorArgs(args); + switch (dec.kind) { + case 'field': + dec.context.addInitializer(function (this: any) { + let getInjection = function (this: any) { + let owner = getOwner(this) || this.container; // fallback to `container` for backwards compat + + assert( + `Attempting to lookup an injected property on an object without a container, ensure that the object was instantiated via a container.`, + Boolean(owner) + ); + + return owner.lookup(`${type}:${name || (dec.context.name as string)}`); + }; + + if (DEBUG) { + DEBUG_INJECTION_FUNCTIONS.set(getInjection, { + type, + name, + }); + } + + Object.defineProperty(this, dec.context.name, { + get: getInjection, + set(value) { + defineProperty(this, dec.context.name as string, null, value); + }, + }); + }); + return; + default: + throw new Error(`unimplemented: injected on ${dec.kind} ${dec.context.name?.toString()}`); + } +} + export default inject; diff --git a/packages/@ember/object/index.ts b/packages/@ember/object/index.ts index f4030c8093f..140f76a6c4f 100644 --- a/packages/@ember/object/index.ts +++ b/packages/@ember/object/index.ts @@ -11,6 +11,11 @@ import { setObservers } from '@ember/-internals/utils'; import type { AnyFn } from '@ember/-internals/utility-types'; import CoreObject from '@ember/object/core'; import Observable from '@ember/object/observable'; +import { + type Decorator, + identifyModernDecoratorArgs, + isModernDecoratorArgs, +} from '@ember/-internals/metal/lib/decorator-util'; export { notifyPropertyChange, @@ -181,6 +186,10 @@ export function action(desc: PropertyDescriptor): ExtendedMethodDecorator; export function action( ...args: ElementDescriptor | [PropertyDescriptor] ): PropertyDescriptor | ExtendedMethodDecorator { + if (isModernDecoratorArgs(args)) { + return action2023(args) as unknown as PropertyDescriptor; + } + let actionFn: object | Function; if (!isElementDescriptor(args)) { @@ -309,3 +318,20 @@ export function observer( }); return func; } + +function action2023(args: Parameters) { + const dec = identifyModernDecoratorArgs(args); + switch (dec.kind) { + case 'method': + dec.context.addInitializer(function (this: any) { + Object.defineProperty( + this, + dec.context.name, + setupAction(this, dec.context.name, dec.value) + ); + }); + break; + default: + throw new Error(`unimplemented: action on ${dec.kind} ${dec.context.name?.toString()}`); + } +} diff --git a/packages/@ember/service/tests/service_test.js b/packages/@ember/service/tests/service_test.js index 6edbb76ab36..041235b7cc7 100644 --- a/packages/@ember/service/tests/service_test.js +++ b/packages/@ember/service/tests/service_test.js @@ -50,7 +50,6 @@ moduleFor( let owner = buildOwner(); class MainService extends Service {} - class Foo extends EmberObject { @inject main; } From 245f8a0dbd0bbadf5fff99ffc3c1942c2dc66ed7 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 6 Mar 2025 15:13:05 -0500 Subject: [PATCH 04/23] fixing message --- packages/@ember/-internals/metal/lib/decorator.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index 5600f552f4b..6fef2621925 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -180,9 +180,11 @@ function computedDecorator2023(args: Parameters, desc: ComputedDescri makeDescriptor(desc, this, dec.context.name as string) ); }); - return; + break; default: - console.log(`unimplemented: injected on ${dec.kind} ${dec.context.name?.toString()}`); + console.log( + `unimplemented: computedDecorator on ${dec.kind} ${dec.context.name?.toString()}` + ); } } From 058d822b45e1316a110a93a1a1b254926f1b63f8 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 6 Mar 2025 15:28:37 -0500 Subject: [PATCH 05/23] fixing arg length check --- packages/@ember/-internals/metal/lib/decorator.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index 6fef2621925..8080c15595c 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -131,7 +131,15 @@ export function makeComputedDecorator( boolean | undefined ]; - return makeDescriptor(desc, target, key, propertyDesc, maybeMeta, isClassicDecorator); + return makeDescriptor( + desc, + args.length, + target, + key, + propertyDesc, + maybeMeta, + isClassicDecorator + ); }; setClassicDecorator(decorator, desc); @@ -143,6 +151,7 @@ export function makeComputedDecorator( function makeDescriptor( desc: ComputedDescriptor, + argsLength: number, target: object, key: string, propertyDesc?: DecoratorPropertyDescriptor, @@ -157,7 +166,7 @@ function makeDescriptor( !COMPUTED_GETTERS.has(propertyDesc.get) ); - let meta = arguments.length === 3 ? metaFor(target) : maybeMeta; + let meta = argsLength === 3 ? metaFor(target) : maybeMeta; desc.setup(target, key, propertyDesc, meta!); let computedDesc: PropertyDescriptor = { @@ -177,7 +186,7 @@ function computedDecorator2023(args: Parameters, desc: ComputedDescri Object.defineProperty( this, dec.context.name, - makeDescriptor(desc, this, dec.context.name as string) + makeDescriptor(desc, 2, this, dec.context.name as string) ); }); break; From c8e4481d7a806fcb137cf5253800a87f3ef9ba56 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sat, 8 Mar 2025 10:07:35 -0500 Subject: [PATCH 06/23] restore assertion --- packages/@ember/-internals/metal/lib/tracked.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index c0a42fd4b90..8800438eb69 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -126,10 +126,18 @@ export function tracked(...args: any[]): ExtendedMethodDecorator | DecoratorProp _meta?: any, isClassicDecorator?: boolean ): DecoratorPropertyDescriptor { - assert( - `You attempted to set a default value for ${key} with the @tracked({ value: 'default' }) syntax. You can only use this syntax with classic classes. For native classes, you can use class initializers: @tracked field = 'default';`, - isClassicDecorator - ); + let args = Array.from(arguments); + if (isModernDecoratorArgs(args)) { + assert( + `You attempted to set a default value for ${args[1].name?.toString()} with the @tracked({ value: 'default' }) syntax. You can only use this syntax with classic classes. For native classes, you can use class initializers: @tracked field = 'default';`, + isClassicDecorator + ); + } else { + assert( + `You attempted to set a default value for ${key} with the @tracked({ value: 'default' }) syntax. You can only use this syntax with classic classes. For native classes, you can use class initializers: @tracked field = 'default';`, + isClassicDecorator + ); + } let fieldDesc = { initializer: initializer || (() => value), From 0560a0cf3eba7eeaed156db0c60e8542b76d830c Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sat, 8 Mar 2025 17:21:32 -0500 Subject: [PATCH 07/23] progress --- .../@ember/-internals/metal/lib/cached.ts | 28 ++++++++++++++++++- .../@ember/-internals/metal/lib/computed.ts | 7 +++-- .../@ember/-internals/metal/lib/decorator.ts | 15 ++++++++-- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/cached.ts b/packages/@ember/-internals/metal/lib/cached.ts index b40ccf4a2de..1503d6ab449 100644 --- a/packages/@ember/-internals/metal/lib/cached.ts +++ b/packages/@ember/-internals/metal/lib/cached.ts @@ -3,6 +3,11 @@ // of @cached, so any changes made to one should also be made to the other import { DEBUG } from '@glimmer/env'; import { createCache, getValue } from '@glimmer/validator'; +import { + type Decorator, + identifyModernDecoratorArgs, + isModernDecoratorArgs, +} from './decorator-util'; /** * @decorator @@ -84,7 +89,7 @@ import { createCache, getValue } from '@glimmer/validator'; the subsequent cache invalidations of the `@cached` properties who were using this `trackedProp`. - Remember that setting tracked data should only be done during initialization, + Remember that setting tracked data should only be done during initialization, or as the result of a user action. Setting tracked data during render (such as in a getter), is not supported. @@ -94,6 +99,10 @@ import { createCache, getValue } from '@glimmer/validator'; @public */ export const cached: MethodDecorator = (...args: any[]) => { + if (isModernDecoratorArgs(args)) { + return cached2023(args) as any; + } + const [target, key, descriptor] = args; // Error on `@cached()`, `@cached(...args)`, and `@cached propName = value;` @@ -123,6 +132,23 @@ export const cached: MethodDecorator = (...args: any[]) => { }; }; +function cached2023(args: Parameters) { + const dec = identifyModernDecoratorArgs(args); + switch (dec.kind) { + case 'getter': { + const caches = new WeakMap(); + return function (this: any) { + if (!caches.has(this)) { + caches.set(this, createCache(dec.value.bind(this))); + } + return getValue(caches.get(this)); + }; + } + default: + throw new Error(`unsupported use of @cached on ${dec.kind} ${dec.context.name?.toString()}`); + } +} + function throwCachedExtraneousParens(): never { throw new Error( 'You attempted to use @cached(), which is not necessary nor supported. Remove the parentheses and you will be good to go!' diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts index 93e74c0cd97..922cffb933d 100644 --- a/packages/@ember/-internals/metal/lib/computed.ts +++ b/packages/@ember/-internals/metal/lib/computed.ts @@ -886,8 +886,11 @@ export function computed( ...args: ElementDescriptor | string[] | ComputedDecoratorKeysAndConfig ): ComputedDecorator | DecoratorPropertyDescriptor | void { if (isModernDecoratorArgs(args)) { - console.log(`ignoring computed on ${args[1].kind} ${args[1].name?.toString()}`); - return; + let decorator = makeComputedDecorator( + new ComputedProperty([]), + ComputedDecoratorImpl + ) as ComputedDecorator; + return decorator(...(args as [any, any])); } assert( diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index 75701e2b0b0..58bbeba98ba 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -128,7 +128,7 @@ export function makeComputedDecorator( string, DecoratorPropertyDescriptor | undefined, Meta | undefined, - boolean | undefined + boolean | undefined, ]; return makeDescriptor( @@ -166,7 +166,7 @@ function makeDescriptor( !COMPUTED_GETTERS.has(propertyDesc.get) ); - let meta = argsLength === 3 ? metaFor(target) : maybeMeta; + let meta = argsLength < 4 ? metaFor(target) : maybeMeta; desc.setup(target, key, propertyDesc, meta!); let computedDesc: PropertyDescriptor = { @@ -190,6 +190,17 @@ function computedDecorator2023(args: Parameters, desc: ComputedDescri ); }); break; + case 'getter': + dec.context.addInitializer(function (this: any) { + Object.defineProperty( + this, + dec.context.name, + makeDescriptor(desc, 2, this, dec.context.name as string, { + get: dec.value as any, + }) + ); + }); + break; default: console.log( `unimplemented: computedDecorator on ${dec.kind} ${dec.context.name?.toString()}` From 9c61bf298b13f983b4206b1fdcd51b90eb51111a Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sat, 8 Mar 2025 17:22:58 -0500 Subject: [PATCH 08/23] progress: 38 failing tests --- packages/@ember/-internals/metal/lib/decorator.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index 58bbeba98ba..2fe0d4b495a 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -201,8 +201,19 @@ function computedDecorator2023(args: Parameters, desc: ComputedDescri ); }); break; + case 'setter': + dec.context.addInitializer(function (this: any) { + Object.defineProperty( + this, + dec.context.name, + makeDescriptor(desc, 2, this, dec.context.name as string, { + set: dec.value as any, + }) + ); + }); + break; default: - console.log( + throw new Error( `unimplemented: computedDecorator on ${dec.kind} ${dec.context.name?.toString()}` ); } From e86df8319c21285c160bd1103d338621b3a7aec2 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sun, 9 Mar 2025 09:06:19 -0400 Subject: [PATCH 09/23] starting to split decorator-time and addInitializer-time parts of computed --- .../@ember/-internals/metal/lib/decorator.ts | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index 2fe0d4b495a..7012f795502 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -131,15 +131,10 @@ export function makeComputedDecorator( boolean | undefined, ]; - return makeDescriptor( - desc, - args.length, - target, - key, - propertyDesc, - maybeMeta, - isClassicDecorator - ); + let meta = args.length < 4 ? metaFor(target) : maybeMeta; + desc.setup(target, key, propertyDesc, meta!); + + return makeDescriptor(desc, key, propertyDesc, isClassicDecorator); }; setClassicDecorator(decorator, desc); @@ -151,11 +146,8 @@ export function makeComputedDecorator( function makeDescriptor( desc: ComputedDescriptor, - argsLength: number, - target: object, key: string, propertyDesc?: DecoratorPropertyDescriptor, - maybeMeta?: Meta, isClassicDecorator?: boolean ): PropertyDescriptor { assert( @@ -166,9 +158,6 @@ function makeDescriptor( !COMPUTED_GETTERS.has(propertyDesc.get) ); - let meta = argsLength < 4 ? metaFor(target) : maybeMeta; - desc.setup(target, key, propertyDesc, meta!); - let computedDesc: PropertyDescriptor = { enumerable: desc.enumerable, configurable: desc.configurable, @@ -183,32 +172,37 @@ function computedDecorator2023(args: Parameters, desc: ComputedDescri switch (dec.kind) { case 'field': dec.context.addInitializer(function (this: any) { + desc.setup(this, dec.context.name as string, undefined, metaFor(this)); Object.defineProperty( this, dec.context.name, - makeDescriptor(desc, 2, this, dec.context.name as string) + makeDescriptor(desc, dec.context.name as string) ); }); break; case 'getter': dec.context.addInitializer(function (this: any) { + let propDesc = { + get: dec.value as any, + }; + desc.setup(this, dec.context.name as string, propDesc, metaFor(this)); Object.defineProperty( this, dec.context.name, - makeDescriptor(desc, 2, this, dec.context.name as string, { - get: dec.value as any, - }) + makeDescriptor(desc, dec.context.name as string, propDesc) ); }); break; case 'setter': dec.context.addInitializer(function (this: any) { + let propDesc = { + set: dec.value as any, + }; + desc.setup(this, dec.context.name as string, propDesc, metaFor(this)); Object.defineProperty( this, dec.context.name, - makeDescriptor(desc, 2, this, dec.context.name as string, { - set: dec.value as any, - }) + makeDescriptor(desc, dec.context.name as string, propDesc) ); }); break; From ff4d29b4e7a74fca1c28ab407ab7d797dbdc28a1 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sun, 9 Mar 2025 12:42:37 -0400 Subject: [PATCH 10/23] progress --- .../@ember/-internals/metal/lib/decorator.ts | 55 +++++++++++-------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index 7012f795502..987a9e1b8fb 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -169,10 +169,19 @@ function makeDescriptor( function computedDecorator2023(args: Parameters, desc: ComputedDescriptor) { const dec = identifyModernDecoratorArgs(args); + + let setup: undefined | ((prototype: any, propDesc: any) => void) = function ( + prototype, + propDesc + ) { + desc.setup(prototype, dec.context.name as string, propDesc, metaFor(prototype)); + setup = undefined; + }; + switch (dec.kind) { case 'field': dec.context.addInitializer(function (this: any) { - desc.setup(this, dec.context.name as string, undefined, metaFor(this)); + setup?.(this.constructor.prototype, undefined); Object.defineProperty( this, dec.context.name, @@ -180,32 +189,32 @@ function computedDecorator2023(args: Parameters, desc: ComputedDescri ); }); break; - case 'getter': + case 'getter': { + let propDesc = { + get: dec.value as any, + }; dec.context.addInitializer(function (this: any) { - let propDesc = { - get: dec.value as any, - }; - desc.setup(this, dec.context.name as string, propDesc, metaFor(this)); - Object.defineProperty( - this, - dec.context.name, - makeDescriptor(desc, dec.context.name as string, propDesc) - ); + setup?.(this.constructor.prototype, propDesc); }); - break; - case 'setter': + return makeDescriptor(desc, dec.context.name as string, propDesc).get; + } + case 'setter': { + let propDesc = { + set: dec.value as any, + }; dec.context.addInitializer(function (this: any) { - let propDesc = { - set: dec.value as any, - }; - desc.setup(this, dec.context.name as string, propDesc, metaFor(this)); - Object.defineProperty( - this, - dec.context.name, - makeDescriptor(desc, dec.context.name as string, propDesc) - ); + setup?.(this.constructor.prototype, propDesc); }); - break; + return makeDescriptor(desc, dec.context.name as string, propDesc).set; + } + case 'method': + assert( + `@computed can only be used on accessors or fields, attempted to use it with ${dec.context.name.toString()} but that was a method. Try converting it to a getter (e.g. \`get ${dec.context.name.toString()}() {}\`)`, + false + ); + // TS knows "assert()" is terminal and will complain about unreachable code if + // I use a break here. ESLint complains if I *don't* use a break here. + // eslint-disable-next-line no-fallthrough default: throw new Error( `unimplemented: computedDecorator on ${dec.kind} ${dec.context.name?.toString()}` From 577dfc21207d6e389d8e769221016bd027e1f8b2 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sun, 9 Mar 2025 13:15:58 -0400 Subject: [PATCH 11/23] adjust action --- packages/@ember/object/index.ts | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/@ember/object/index.ts b/packages/@ember/object/index.ts index e70846456ca..294f8389c43 100644 --- a/packages/@ember/object/index.ts +++ b/packages/@ember/object/index.ts @@ -321,17 +321,19 @@ export function observer( function action2023(args: Parameters) { const dec = identifyModernDecoratorArgs(args); - switch (dec.kind) { - case 'method': - dec.context.addInitializer(function (this: any) { - Object.defineProperty( - this, - dec.context.name, - setupAction(this, dec.context.name, dec.value) - ); - }); - break; - default: - throw new Error(`unimplemented: action on ${dec.kind} ${dec.context.name?.toString()}`); - } + assert( + 'The @action decorator must be applied to methods when used in native classes', + dec.kind === 'method' + ); + let needsSetup = true; + dec.context.addInitializer(function (this: any) { + if (needsSetup) { + Object.defineProperty( + this.constructor.prototype, + dec.context.name, + setupAction(this.constructor.prototype, dec.context.name, dec.value) + ); + needsSetup = false; + } + }); } From e4242d38de59e3340c95932dc5e7cdf8761749c3 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sun, 9 Mar 2025 15:56:11 -0400 Subject: [PATCH 12/23] progress --- .../@ember/-internals/metal/lib/decorator.ts | 64 ++++++++++++------- .../metal/tests/computed_decorator_test.js | 1 + .../-internals/utils/lib/lookup-descriptor.ts | 14 +++- packages/@ember/object/index.ts | 14 ++-- 4 files changed, 62 insertions(+), 31 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index 987a9e1b8fb..ba636c3163c 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -7,6 +7,7 @@ import { identifyModernDecoratorArgs, isModernDecoratorArgs, } from './decorator-util'; +import { findDescriptor } from '@ember/-internals/utils/lib/lookup-descriptor'; export type DecoratorPropertyDescriptor = (PropertyDescriptor & { initializer?: any }) | undefined; @@ -167,45 +168,60 @@ function makeDescriptor( return computedDesc; } +function once() { + let needsToRun = true; + return function (fn: () => void): void { + if (needsToRun) { + fn(); + needsToRun = false; + } + }; +} + function computedDecorator2023(args: Parameters, desc: ComputedDescriptor) { const dec = identifyModernDecoratorArgs(args); - - let setup: undefined | ((prototype: any, propDesc: any) => void) = function ( - prototype, - propDesc - ) { - desc.setup(prototype, dec.context.name as string, propDesc, metaFor(prototype)); - setup = undefined; - }; + let setup = once(); switch (dec.kind) { case 'field': dec.context.addInitializer(function (this: any) { - setup?.(this.constructor.prototype, undefined); + setup(() => { + desc.setup( + this.constructor.prototype, + dec.context.name as string, + undefined, + metaFor(this.constructor.prototype) + ); + }); Object.defineProperty( this, dec.context.name, makeDescriptor(desc, dec.context.name as string) ); }); - break; + return undefined; + case 'setter': case 'getter': { - let propDesc = { - get: dec.value as any, - }; - dec.context.addInitializer(function (this: any) { - setup?.(this.constructor.prototype, propDesc); - }); - return makeDescriptor(desc, dec.context.name as string, propDesc).get; - } - case 'setter': { - let propDesc = { - set: dec.value as any, - }; dec.context.addInitializer(function (this: any) { - setup?.(this.constructor.prototype, propDesc); + setup(() => { + let found = findDescriptor(this, dec.context.name); + if (!found) { + return; + } + desc.setup( + found.object, + dec.context.name as string, + found.descriptor, + metaFor(found.object) + ); + Object.defineProperty( + found.object, + dec.context.name, + makeDescriptor(desc, dec.context.name as string, found.descriptor) + ); + }); }); - return makeDescriptor(desc, dec.context.name as string, propDesc).set; + return undefined; } case 'method': assert( diff --git a/packages/@ember/-internals/metal/tests/computed_decorator_test.js b/packages/@ember/-internals/metal/tests/computed_decorator_test.js index 1ae295e7cc6..b37d5d186e5 100644 --- a/packages/@ember/-internals/metal/tests/computed_decorator_test.js +++ b/packages/@ember/-internals/metal/tests/computed_decorator_test.js @@ -71,6 +71,7 @@ moduleFor( ['@test computed property can be defined and accessed on a class constructor'](assert) { let count = 0; + debugger; class Obj { static bar = 123; diff --git a/packages/@ember/-internals/utils/lib/lookup-descriptor.ts b/packages/@ember/-internals/utils/lib/lookup-descriptor.ts index 395dea6ebde..66e67db9d77 100644 --- a/packages/@ember/-internals/utils/lib/lookup-descriptor.ts +++ b/packages/@ember/-internals/utils/lib/lookup-descriptor.ts @@ -1,11 +1,21 @@ -export default function lookupDescriptor(obj: object, keyName: string | symbol) { +export function findDescriptor( + obj: object, + keyName: string | symbol +): { object: object; descriptor: PropertyDescriptor } | null { let current: object | null = obj; do { let descriptor = Object.getOwnPropertyDescriptor(current, keyName); if (descriptor !== undefined) { - return descriptor; + return { descriptor, object: current }; } current = Object.getPrototypeOf(current); } while (current !== null); return null; } + +export default function lookupDescriptor( + obj: object, + keyName: string | symbol +): PropertyDescriptor | null { + return findDescriptor(obj, keyName)?.descriptor ?? null; +} diff --git a/packages/@ember/object/index.ts b/packages/@ember/object/index.ts index 294f8389c43..25f5b6a58fa 100644 --- a/packages/@ember/object/index.ts +++ b/packages/@ember/object/index.ts @@ -16,6 +16,7 @@ import { identifyModernDecoratorArgs, isModernDecoratorArgs, } from '@ember/-internals/metal/lib/decorator-util'; +import { findDescriptor } from '@ember/-internals/utils/lib/lookup-descriptor'; export { notifyPropertyChange, @@ -328,11 +329,14 @@ function action2023(args: Parameters) { let needsSetup = true; dec.context.addInitializer(function (this: any) { if (needsSetup) { - Object.defineProperty( - this.constructor.prototype, - dec.context.name, - setupAction(this.constructor.prototype, dec.context.name, dec.value) - ); + let found = findDescriptor(this, dec.context.name); + if (found?.object) { + Object.defineProperty( + found.object, + dec.context.name, + setupAction(found.object, dec.context.name, dec.value) + ); + } needsSetup = false; } }); From 03e53fb74321a54c8ba9e3995758a906df5322b4 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sun, 9 Mar 2025 16:04:32 -0400 Subject: [PATCH 13/23] progress --- packages/@ember/object/compat.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/@ember/object/compat.ts b/packages/@ember/object/compat.ts index 39265d27b66..dece38a0eaf 100644 --- a/packages/@ember/object/compat.ts +++ b/packages/@ember/object/compat.ts @@ -6,6 +6,10 @@ import { setClassicDecorator, } from '@ember/-internals/metal'; import type { ElementDescriptor } from '@ember/-internals/metal'; +import { + identifyModernDecoratorArgs, + isModernDecoratorArgs, +} from '@ember/-internals/metal/lib/decorator-util'; import { assert } from '@ember/debug'; import type { UpdatableTag } from '@glimmer/validator'; import { consumeTag, tagFor, track, updateTag } from '@glimmer/validator'; @@ -139,6 +143,25 @@ export function dependentKeyCompat( ); return wrapGetterSetter(target, key, desc); + } else if (isModernDecoratorArgs(args)) { + const dec = identifyModernDecoratorArgs(args); + assert( + 'The @dependentKeyCompat decorator must be applied to getters/setters when used in native classes', + dec.kind === 'getter' + ); + return function (this: any) { + let propertyTag = tagFor(this, dec.context.name as string) as UpdatableTag; + let ret; + + let tag = track(() => { + ret = dec.value.call(this); + }); + + updateTag(propertyTag, tag); + consumeTag(tag); + + return ret; + }; } else { const desc = args[0]; From 81dbb6c358346a51ada733ea496a9ea8fadd2b8e Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sun, 9 Mar 2025 16:23:52 -0400 Subject: [PATCH 14/23] all passing tests! --- packages/@ember/-internals/metal/index.ts | 1 + packages/@ember/-internals/metal/lib/alias.ts | 4 +-- .../@ember/-internals/metal/lib/decorator.ts | 6 ++++ .../object/lib/computed/computed_macros.ts | 32 +++++++++---------- .../lib/computed/reduce_computed_macros.ts | 28 ++++++++-------- 5 files changed, 39 insertions(+), 32 deletions(-) diff --git a/packages/@ember/-internals/metal/index.ts b/packages/@ember/-internals/metal/index.ts index 608bfa75037..9895f85b473 100644 --- a/packages/@ember/-internals/metal/index.ts +++ b/packages/@ember/-internals/metal/index.ts @@ -47,6 +47,7 @@ export { ComputedDescriptor, type ElementDescriptor, isElementDescriptor, + isDecoratorCall, nativeDescDecorator, descriptorForDecorator, descriptorForProperty, diff --git a/packages/@ember/-internals/metal/lib/alias.ts b/packages/@ember/-internals/metal/lib/alias.ts index af955f3cb12..aa217d2ff76 100644 --- a/packages/@ember/-internals/metal/lib/alias.ts +++ b/packages/@ember/-internals/metal/lib/alias.ts @@ -16,7 +16,7 @@ import type { ExtendedMethodDecorator } from './decorator'; import { ComputedDescriptor, descriptorForDecorator, - isElementDescriptor, + isDecoratorCall, makeComputedDecorator, } from './decorator'; import { defineProperty } from './properties'; @@ -28,7 +28,7 @@ export type AliasDecorator = ExtendedMethodDecorator & PropertyDecorator & Alias export default function alias(altKey: string): AliasDecorator { assert( 'You attempted to use @alias as a decorator directly, but it requires a `altKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); // SAFETY: We passed in the impl for this class diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index ba636c3163c..54f32ed4706 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -42,6 +42,12 @@ export function isElementDescriptor(args: unknown[]): args is ElementDescriptor ); } +export function isDecoratorCall( + args: unknown[] +): args is ElementDescriptor | Parameters { + return isElementDescriptor(args) || isModernDecoratorArgs(args); +} + export function nativeDescDecorator(propertyDesc: PropertyDescriptor) { let decorator = function () { return propertyDesc; diff --git a/packages/@ember/object/lib/computed/computed_macros.ts b/packages/@ember/object/lib/computed/computed_macros.ts index 0c71a2967e0..bdb76bdc6c9 100644 --- a/packages/@ember/object/lib/computed/computed_macros.ts +++ b/packages/@ember/object/lib/computed/computed_macros.ts @@ -1,4 +1,4 @@ -import { computed, isElementDescriptor, alias, expandProperties } from '@ember/-internals/metal'; +import { computed, isDecoratorCall, alias, expandProperties } from '@ember/-internals/metal'; import { get, set } from '@ember/object'; import type { DeprecationOptions } from '@ember/debug'; import { assert, deprecate } from '@ember/debug'; @@ -33,7 +33,7 @@ function generateComputedWithPredicate(name: string, predicate: (value: unknown) assert( `You attempted to use @${name} as a decorator directly, but it requires at least one dependent key parameter`, - !isElementDescriptor(properties) + !isDecoratorCall(properties) ); let dependentKeys = expandPropertiesToArray(name, properties); @@ -98,7 +98,7 @@ function generateComputedWithPredicate(name: string, predicate: (value: unknown) export function empty(dependentKey: string) { assert( 'You attempted to use @empty as a decorator directly, but it requires a `dependentKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(`${dependentKey}.length`, function () { @@ -144,7 +144,7 @@ export function empty(dependentKey: string) { export function notEmpty(dependentKey: string) { assert( 'You attempted to use @notEmpty as a decorator directly, but it requires a `dependentKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(`${dependentKey}.length`, function () { @@ -187,7 +187,7 @@ export function notEmpty(dependentKey: string) { export function none(dependentKey: string) { assert( 'You attempted to use @none as a decorator directly, but it requires a `dependentKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(dependentKey, function () { @@ -229,7 +229,7 @@ export function none(dependentKey: string) { export function not(dependentKey: string) { assert( 'You attempted to use @not as a decorator directly, but it requires a `dependentKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(dependentKey, function () { @@ -277,7 +277,7 @@ export function not(dependentKey: string) { export function bool(dependentKey: string) { assert( 'You attempted to use @bool as a decorator directly, but it requires a `dependentKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(dependentKey, function () { @@ -323,7 +323,7 @@ export function bool(dependentKey: string) { export function match(dependentKey: string, regexp: RegExp) { assert( 'You attempted to use @match as a decorator directly, but it requires `dependentKey` and `regexp` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(dependentKey, function () { @@ -369,7 +369,7 @@ export function match(dependentKey: string, regexp: RegExp) { export function equal(dependentKey: string, value: unknown) { assert( 'You attempted to use @equal as a decorator directly, but it requires `dependentKey` and `value` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(dependentKey, function () { @@ -414,7 +414,7 @@ export function equal(dependentKey: string, value: unknown) { export function gt(dependentKey: string, value: number) { assert( 'You attempted to use @gt as a decorator directly, but it requires `dependentKey` and `value` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(dependentKey, function () { @@ -459,7 +459,7 @@ export function gt(dependentKey: string, value: number) { export function gte(dependentKey: string, value: number) { assert( 'You attempted to use @gte as a decorator directly, but it requires `dependentKey` and `value` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(dependentKey, function () { @@ -504,7 +504,7 @@ export function gte(dependentKey: string, value: number) { export function lt(dependentKey: string, value: number) { assert( 'You attempted to use @lt as a decorator directly, but it requires `dependentKey` and `value` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(dependentKey, function () { @@ -549,7 +549,7 @@ export function lt(dependentKey: string, value: number) { export function lte(dependentKey: string, value: number) { assert( 'You attempted to use @lte as a decorator directly, but it requires `dependentKey` and `value` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(dependentKey, function () { @@ -723,7 +723,7 @@ export const or = generateComputedWithPredicate('or', (value) => !value); export function oneWay(dependentKey: string) { assert( 'You attempted to use @oneWay as a decorator directly, but it requires a `dependentKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return alias(dependentKey).oneWay() as PropertyDecorator; @@ -787,7 +787,7 @@ export function oneWay(dependentKey: string) { export function readOnly(dependentKey: string) { assert( 'You attempted to use @readOnly as a decorator directly, but it requires a `dependentKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return alias(dependentKey).readOnly() as PropertyDecorator; @@ -831,7 +831,7 @@ export function readOnly(dependentKey: string) { export function deprecatingAlias(dependentKey: string, options: DeprecationOptions) { assert( 'You attempted to use @deprecatingAlias as a decorator directly, but it requires `dependentKey` and `options` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return computed(dependentKey, { diff --git a/packages/@ember/object/lib/computed/reduce_computed_macros.ts b/packages/@ember/object/lib/computed/reduce_computed_macros.ts index 525c9611746..679c7b4ca99 100644 --- a/packages/@ember/object/lib/computed/reduce_computed_macros.ts +++ b/packages/@ember/object/lib/computed/reduce_computed_macros.ts @@ -3,7 +3,7 @@ */ import { DEBUG } from '@glimmer/env'; import { assert } from '@ember/debug'; -import { autoComputed, isElementDescriptor } from '@ember/-internals/metal'; +import { autoComputed, isDecoratorCall } from '@ember/-internals/metal'; import { computed, get } from '@ember/object'; import { compare } from '@ember/utils'; import EmberArray, { A as emberA, uniqBy as uniqByArray } from '@ember/array'; @@ -104,7 +104,7 @@ function multiArrayMacro( export function sum(dependentKey: string) { assert( 'You attempted to use @sum as a decorator directly, but it requires a `dependentKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return reduceMacro(dependentKey, (sum: number, item: number) => sum + item, 0, 'sum'); @@ -168,7 +168,7 @@ export function sum(dependentKey: string) { export function max(dependentKey: string) { assert( 'You attempted to use @max as a decorator directly, but it requires a `dependentKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return reduceMacro(dependentKey, (max, item) => Math.max(max, item), -Infinity, 'max'); @@ -231,7 +231,7 @@ export function max(dependentKey: string) { export function min(dependentKey: string) { assert( 'You attempted to use @min as a decorator directly, but it requires a `dependentKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); return reduceMacro(dependentKey, (min, item) => Math.min(min, item), Infinity, 'min'); @@ -328,7 +328,7 @@ export function map( ): PropertyDecorator { assert( 'You attempted to use @map as a decorator directly, but it requires atleast `dependentKey` and `callback` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); assert( @@ -412,7 +412,7 @@ export function map( export function mapBy(dependentKey: string, propertyKey: string) { assert( 'You attempted to use @mapBy as a decorator directly, but it requires `dependentKey` and `propertyKey` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); assert( @@ -554,7 +554,7 @@ export function filter( ): PropertyDecorator { assert( 'You attempted to use @filter as a decorator directly, but it requires atleast `dependentKey` and `callback` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); assert( @@ -636,7 +636,7 @@ export function filter( export function filterBy(dependentKey: string, propertyKey: string, value?: unknown) { assert( 'You attempted to use @filterBy as a decorator directly, but it requires atleast `dependentKey` and `propertyKey` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); assert( @@ -696,7 +696,7 @@ export function uniq( ): PropertyDecorator { assert( 'You attempted to use @uniq/@union as a decorator directly, but it requires atleast one dependent key parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); let args = [dependentKey, ...additionalDependentKeys]; @@ -765,7 +765,7 @@ export function uniq( export function uniqBy(dependentKey: string, propertyKey: string) { assert( 'You attempted to use @uniqBy as a decorator directly, but it requires `dependentKey` and `propertyKey` parameters', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); assert( @@ -864,7 +864,7 @@ export let union = uniq; export function intersect(dependentKey: string, ...additionalDependentKeys: string[]) { assert( 'You attempted to use @intersect as a decorator directly, but it requires atleast one dependent key parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); let args = [dependentKey, ...additionalDependentKeys]; @@ -953,7 +953,7 @@ export function intersect(dependentKey: string, ...additionalDependentKeys: stri export function setDiff(setAProperty: string, setBProperty: string) { assert( 'You attempted to use @setDiff as a decorator directly, but it requires atleast one dependent key parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); assert('`setDiff` computed macro requires exactly two dependent arrays.', arguments.length === 2); @@ -1011,7 +1011,7 @@ export function setDiff(setAProperty: string, setBProperty: string) { export function collect(dependentKey: string, ...additionalDependentKeys: string[]) { assert( 'You attempted to use @collect as a decorator directly, but it requires atleast one dependent key parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); let dependentKeys = [dependentKey, ...additionalDependentKeys]; @@ -1188,7 +1188,7 @@ export function sort( ): PropertyDecorator { assert( 'You attempted to use @sort as a decorator directly, but it requires atleast an `itemsKey` parameter', - !isElementDescriptor(Array.prototype.slice.call(arguments)) + !isDecoratorCall(Array.prototype.slice.call(arguments)) ); if (DEBUG) { From 55832e5b65c65f7ba73102e2da5165769b456542 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sun, 9 Mar 2025 16:25:39 -0400 Subject: [PATCH 15/23] lints --- packages/@ember/-internals/metal/lib/tracked.ts | 6 +++++- .../-internals/metal/tests/computed_decorator_test.js | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index 8800438eb69..b107a787b38 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -8,7 +8,11 @@ import { CHAIN_PASS_THROUGH } from './chain-tags'; import type { ExtendedMethodDecorator, DecoratorPropertyDescriptor } from './decorator'; import { COMPUTED_SETTERS, isElementDescriptor, setClassicDecorator } from './decorator'; import { SELF_TAG } from './tags'; -import { Decorator, identifyModernDecoratorArgs, isModernDecoratorArgs } from './decorator-util'; +import { + type Decorator, + identifyModernDecoratorArgs, + isModernDecoratorArgs, +} from './decorator-util'; /** @decorator diff --git a/packages/@ember/-internals/metal/tests/computed_decorator_test.js b/packages/@ember/-internals/metal/tests/computed_decorator_test.js index b37d5d186e5..1ae295e7cc6 100644 --- a/packages/@ember/-internals/metal/tests/computed_decorator_test.js +++ b/packages/@ember/-internals/metal/tests/computed_decorator_test.js @@ -71,7 +71,6 @@ moduleFor( ['@test computed property can be defined and accessed on a class constructor'](assert) { let count = 0; - debugger; class Obj { static bar = 123; From d871090e744d8c53af354598f16d27f1e1fd7c8b Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sun, 9 Mar 2025 16:28:09 -0400 Subject: [PATCH 16/23] keep typescript thinking we're using legacy decorators for now --- tsconfig/compiler-options.json | 1 + 1 file changed, 1 insertion(+) diff --git a/tsconfig/compiler-options.json b/tsconfig/compiler-options.json index 2f862e5c0f8..91afd83639d 100644 --- a/tsconfig/compiler-options.json +++ b/tsconfig/compiler-options.json @@ -9,6 +9,7 @@ "rootDir": "../packages", // Environment Configuration + "experimentalDecorators": true, "moduleResolution": "bundler", // Enhance Strictness From 2dac217fe9c5391afa9648e91bf98387f302e35b Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sun, 9 Mar 2025 16:55:07 -0400 Subject: [PATCH 17/23] run stable decorators variant in ci --- .github/workflows/ci.yml | 9 +++++++++ .../-internals/metal/tests/tracked/validation_test.js | 4 +++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1dfe1e05f3c..9e955565192 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -100,18 +100,27 @@ jobs: - name: "Production build, with optional features" BUILD: "production" ENABLE_OPTIONAL_FEATURES: "true" + - name: "Stable decorators" + VITE_STABLE_DECORATORS: "true" steps: - uses: actions/checkout@v4 - uses: ./.github/actions/setup - name: build run: pnpm vite build --mode=${{ matrix.BUILD || 'development' }} + env: + ALL_DEPRECATIONS_ENABLED: ${{ matrix.ALL_DEPRECATIONS_ENABLED }} + OVERRIDE_DEPRECATION_VERSION: ${{ matrix.OVERRIDE_DEPRECATION_VERSION }} + ENABLE_OPTIONAL_FEATURES: ${{ matrix.ENABLE_OPTIONAL_FEATURES }} + RAISE_ON_DEPRECATION: ${{ matrix.RAISE_ON_DEPRECATION }} + VITE_STABLE_DECORATORS: ${{ matrix.VITE_STABLE_DECORATORS }} - name: test env: ALL_DEPRECATIONS_ENABLED: ${{ matrix.ALL_DEPRECATIONS_ENABLED }} OVERRIDE_DEPRECATION_VERSION: ${{ matrix.OVERRIDE_DEPRECATION_VERSION }} ENABLE_OPTIONAL_FEATURES: ${{ matrix.ENABLE_OPTIONAL_FEATURES }} RAISE_ON_DEPRECATION: ${{ matrix.RAISE_ON_DEPRECATION }} + VITE_STABLE_DECORATORS: ${{ matrix.VITE_STABLE_DECORATORS }} run: pnpm test diff --git a/packages/@ember/-internals/metal/tests/tracked/validation_test.js b/packages/@ember/-internals/metal/tests/tracked/validation_test.js index f2d466238ab..e2eef6ed310 100644 --- a/packages/@ember/-internals/metal/tests/tracked/validation_test.js +++ b/packages/@ember/-internals/metal/tests/tracked/validation_test.js @@ -43,7 +43,9 @@ moduleFor( assert.equal(validateTag(tag, snapshot), true); } - [`@test autotracking should work with initializers`](assert) { + [`@test autotracking should work with initializers (${import.meta.env.VITE_STABLE_DECORATORS ? 'stable' : 'legacy'} decorators)`]( + assert + ) { class Tracked { @tracked first = `first: ${this.second}`; @tracked second = 'second'; From ae9a72960357310b04d572461cb8400239ef901c Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sun, 9 Mar 2025 18:32:22 -0400 Subject: [PATCH 18/23] don't try to treeshake test suite Rollup was actually dropping a side-effect that was intentionally part of a test. --- vite.config.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/vite.config.mjs b/vite.config.mjs index b39ec43d6be..57615cfea36 100644 --- a/vite.config.mjs +++ b/vite.config.mjs @@ -22,6 +22,7 @@ export default defineConfig(({ mode }) => { const build = { rollupOptions: { preserveEntrySignatures: 'strict', + treeshake: false, output: { preserveModules: true, }, From b41f58da92dc33bd5cd0e91df89c41b5c75dcde3 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 10 Mar 2025 12:14:05 -0400 Subject: [PATCH 19/23] implement accessor form of tracked --- .../@ember/-internals/metal/lib/tracked.ts | 11 +++ pnpm-lock.yaml | 6 ++ smoke-tests/app-template/ember-cli-build.js | 2 +- .../scenarios/modern-decorator-test.ts | 75 +++++++++++++++++++ smoke-tests/scenarios/package.json | 2 + 5 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 smoke-tests/scenarios/modern-decorator-test.ts diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index b107a787b38..ce9097b4c7d 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -239,6 +239,17 @@ function tracked2023(args: Parameters) { ); }); return; + case 'accessor': + return { + get(this: object) { + consumeTag(tagFor(this, dec.context.name)); + return dec.value.get.call(this); + }, + set(this: object, value: unknown) { + dirtyTagFor(this, dec.context.name); + return dec.value.set.call(this, value); + }, + }; default: throw new Error(`unimplemented: tracked on ${dec.kind} ${dec.context.name?.toString()}`); } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 454414f68bd..932c9aacc6d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1825,6 +1825,9 @@ importers: smoke-tests/scenarios: devDependencies: + '@babel/plugin-proposal-decorators': + specifier: ^7.25.9 + version: 7.25.9(@babel/core@7.26.9) '@embroider/compat': specifier: npm:@embroider/compat@latest version: 3.8.3(@embroider/core@3.5.2) @@ -1846,6 +1849,9 @@ importers: '@types/node': specifier: ^20.12.7 version: 20.17.19 + ember-template-imports: + specifier: ^4.3.0 + version: 4.3.0 qunit: specifier: ^2.20.1 version: 2.24.1 diff --git a/smoke-tests/app-template/ember-cli-build.js b/smoke-tests/app-template/ember-cli-build.js index 77e5ad90726..5f24e71d09b 100644 --- a/smoke-tests/app-template/ember-cli-build.js +++ b/smoke-tests/app-template/ember-cli-build.js @@ -5,7 +5,7 @@ const { maybeEmbroider } = require('@embroider/test-setup'); module.exports = function (defaults) { const app = new EmberApp(defaults, { - // Add options here + /* SCENARIO_INSERTION_TARGET */ }); // Use `app.import` to add additional libraries to the generated diff --git a/smoke-tests/scenarios/modern-decorator-test.ts b/smoke-tests/scenarios/modern-decorator-test.ts new file mode 100644 index 00000000000..4969a35dc18 --- /dev/null +++ b/smoke-tests/scenarios/modern-decorator-test.ts @@ -0,0 +1,75 @@ +import { appScenarios } from './scenarios'; +import type { PreparedApp } from 'scenario-tester'; +import * as QUnit from 'qunit'; +const { module: Qmodule, test } = QUnit; + +appScenarios + .map('modern-decorators', (project) => { + project.files['ember-cli-build.js'] = project.files['ember-cli-build.js'].replace( + '/* SCENARIO_INSERTION_TARGET */', + ` + 'ember-cli-babel': { + disableDecoratorTransforms: true, + }, + babel: { + plugins: [ + [require.resolve('@babel/plugin-proposal-decorators'), { version: '2023-11' }], + ], + },` + ); + + project.linkDevDependency('ember-template-imports', { baseDir: __dirname }) + project.linkDevDependency('@babel/plugin-proposal-decorators', { baseDir: __dirname }) + + project.mergeFiles({ + tests: { + unit: { + 'tracked-accessor-test.gjs': ` + import { module, test } from 'qunit'; + import { setupRenderingTest } from 'ember-qunit'; + import { render, click } from '@ember/test-helpers'; + import { on } from '@ember/modifier'; + import { tracked } from '@glimmer/tracking'; + import Component from '@glimmer/component'; + + module('Unit | tracked-accessor', function(hooks) { + setupRenderingTest(hooks); + + test('interactive update', async function(assert) { + class Example extends Component { + @tracked accessor count = 0; + inc = () => { this.count++ }; + + + } + + await render(); + assert.dom('.example span').hasText('0'); + await click('.example button'); + assert.dom('.example span').hasText('1'); + }); + }); + ` + }, + }, + }); + }) + .forEachScenario((scenario) => { + Qmodule(scenario.name, function (hooks) { + let app: PreparedApp; + hooks.before(async () => { + app = await scenario.prepare(); + }); + + test(`ember test`, async function (assert) { + let result = await app.execute(`ember test`); + assert.equal(result.exitCode, 0, result.output); + }); + }); + }); diff --git a/smoke-tests/scenarios/package.json b/smoke-tests/scenarios/package.json index 6dde37efa2f..47a0b6986d0 100644 --- a/smoke-tests/scenarios/package.json +++ b/smoke-tests/scenarios/package.json @@ -2,6 +2,7 @@ "name": "ember-source-scenarios", "private": true, "devDependencies": { + "@babel/plugin-proposal-decorators": "^7.25.9", "@embroider/compat": "npm:@embroider/compat@latest", "@embroider/core": "npm:@embroider/core@latest", "@embroider/webpack": "npm:@embroider/webpack@latest", @@ -9,6 +10,7 @@ "@swc/core": "^1.4.17", "@swc/types": "^0.1.6", "@types/node": "^20.12.7", + "ember-template-imports": "^4.3.0", "qunit": "^2.20.1", "scenario-tester": "^4.0.0", "typescript": "5.1", From a6f83d130ebf1c2dee4494a5c8f1397e1b573df0 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 11 Mar 2025 13:40:10 -0400 Subject: [PATCH 20/23] implementing @service accessor and fixing setter in modern field @service --- package.json | 2 +- .../-internals/metal/lib/injected_property.ts | 50 +++--- packages/@ember/service/tests/service_test.js | 35 +++++ .../scenarios/modern-decorator-test.ts | 144 ++++++++++++++---- 4 files changed, 182 insertions(+), 49 deletions(-) diff --git a/package.json b/package.json index b2c4d790e51..d6c49ab2cbc 100644 --- a/package.json +++ b/package.json @@ -402,4 +402,4 @@ } }, "packageManager": "pnpm@10.5.0" -} +} \ No newline at end of file diff --git a/packages/@ember/-internals/metal/lib/injected_property.ts b/packages/@ember/-internals/metal/lib/injected_property.ts index a07d76c00f6..8382901df49 100644 --- a/packages/@ember/-internals/metal/lib/injected_property.ts +++ b/packages/@ember/-internals/metal/lib/injected_property.ts @@ -99,37 +99,47 @@ function inject( function inject2023(type: string, name: string | undefined, args: Parameters) { const dec = identifyModernDecoratorArgs(args); - switch (dec.kind) { - case 'field': - dec.context.addInitializer(function (this: any) { - let getInjection = function (this: any) { - let owner = getOwner(this) || this.container; // fallback to `container` for backwards compat - assert( - `Attempting to lookup an injected property on an object without a container, ensure that the object was instantiated via a container.`, - Boolean(owner) - ); + function getInjection(this: any) { + let owner = getOwner(this) || this.container; // fallback to `container` for backwards compat - return owner.lookup(`${type}:${name || (dec.context.name as string)}`); - }; + assert( + `Attempting to lookup an injected property on an object without a container, ensure that the object was instantiated via a container.`, + Boolean(owner) + ); - if (DEBUG) { - DEBUG_INJECTION_FUNCTIONS.set(getInjection, { - type, - name, - }); - } + return owner.lookup(`${type}:${name || (dec.context.name as string)}`); + } + if (DEBUG) { + DEBUG_INJECTION_FUNCTIONS.set(getInjection, { + type, + name, + }); + } + + switch (dec.kind) { + case 'field': + dec.context.addInitializer(function (this: any) { Object.defineProperty(this, dec.context.name, { get: getInjection, - set(value) { - defineProperty(this, dec.context.name as string, null, value); + set(this: object, value: unknown) { + Object.defineProperty(this, dec.context.name, { value }); }, }); }); return; + case 'accessor': + return { + get: getInjection, + set(this: object, value: unknown) { + Object.defineProperty(this, dec.context.name, { value }); + }, + }; default: - throw new Error(`unimplemented: injected on ${dec.kind} ${dec.context.name?.toString()}`); + throw new Error( + `The @service decorator does not support ${dec.kind} ${dec.context.name?.toString()}` + ); } } diff --git a/packages/@ember/service/tests/service_test.js b/packages/@ember/service/tests/service_test.js index 041235b7cc7..5463da6383e 100644 --- a/packages/@ember/service/tests/service_test.js +++ b/packages/@ember/service/tests/service_test.js @@ -106,5 +106,40 @@ moduleFor( runDestroy(owner); } + + ['@test can be replaced by assignment'](assert) { + let owner = buildOwner(); + + class MainService extends Service {} + + class Foo extends EmberObject { + @service main; + } + + owner.register('service:main', MainService); + owner.register('foo:main', Foo); + + let foo = owner.lookup('foo:main'); + let replacement = {}; + foo.main = replacement; + assert.strictEqual(foo.main, replacement, 'replaced'); + + runDestroy(owner); + } + + ['@test throws when used in wrong syntactic position'](assert) { + assert.throws(() => { + // eslint-disable-next-line no-unused-vars + class Foo extends EmberObject { + @service main() {} + } + }, /The @service decorator does not support method main/); + + assert.throws(() => { + @service + // eslint-disable-next-line no-unused-vars + class Foo extends EmberObject {} + }, /The @service decorator does not support class Foo/); + } } ); diff --git a/smoke-tests/scenarios/modern-decorator-test.ts b/smoke-tests/scenarios/modern-decorator-test.ts index 4969a35dc18..96633426263 100644 --- a/smoke-tests/scenarios/modern-decorator-test.ts +++ b/smoke-tests/scenarios/modern-decorator-test.ts @@ -22,39 +22,127 @@ appScenarios project.linkDevDependency('@babel/plugin-proposal-decorators', { baseDir: __dirname }) project.mergeFiles({ + app: { + services: { + 'my-example.js': ` + import Service from '@ember/service'; + export default class MyExample extends Service { + message = 'Message from MyExample'; + + constructor(...args) { + super(...args); + if (globalThis.myExampleTracker) { + globalThis.myExampleTracker.initialized = true; + } + } + } + ` + } + }, tests: { unit: { 'tracked-accessor-test.gjs': ` - import { module, test } from 'qunit'; - import { setupRenderingTest } from 'ember-qunit'; - import { render, click } from '@ember/test-helpers'; - import { on } from '@ember/modifier'; - import { tracked } from '@glimmer/tracking'; - import Component from '@glimmer/component'; - - module('Unit | tracked-accessor', function(hooks) { - setupRenderingTest(hooks); - - test('interactive update', async function(assert) { - class Example extends Component { - @tracked accessor count = 0; - inc = () => { this.count++ }; - - - } + import { module, test } from 'qunit'; + import { setupRenderingTest } from 'ember-qunit'; + import { render, click } from '@ember/test-helpers'; + import { on } from '@ember/modifier'; + import { tracked } from '@glimmer/tracking'; + import Component from '@glimmer/component'; + + module('Unit | tracked-accessor', function(hooks) { + setupRenderingTest(hooks); + + test('interactive update', async function(assert) { + class Example extends Component { + @tracked accessor count = 0; + inc = () => { this.count++ }; + + + } + + await render(); + assert.dom('.example span').hasText('0'); + await click('.example button'); + assert.dom('.example span').hasText('1'); + }); + }); + `, + 'service-accessor-test.gjs': ` + import { module, test } from 'qunit'; + import { setupRenderingTest } from 'ember-qunit'; + import { render, click } from '@ember/test-helpers'; + import { on } from '@ember/modifier'; + import { tracked } from '@glimmer/tracking'; + import Service, { service } from '@ember/service'; + import Component from '@glimmer/component'; + + module('Unit | service-accessor', function(hooks) { + setupRenderingTest(hooks); + + test('service is available', async function(assert) { + class Example extends Component { + @service accessor myExample; + + + } + + await render(); + assert.dom('.example').hasText('Message from MyExample'); + }); + + test('service is lazy', async function(assert) { + class InitTracker { + @tracked accessor initialized = false; + } + globalThis.myExampleTracker = new InitTracker(); + class Example extends Component { + @service accessor myExample; + @tracked accessor reveal = false; + + revealIt = () => { this.reveal = true } + + + } + + await render(); + assert.dom('.example span').hasText('Service Not Initialized') + await click('.example button'); + assert.dom('.example span').hasText('Service Initialized') + }); - await render(); - assert.dom('.example span').hasText('0'); - await click('.example button'); - assert.dom('.example span').hasText('1'); + test('service can be replaced by assignment', async function(assert) { + class ReplacementTest extends Service { + @service accessor myExample; + } + this.owner.register('service:replacement-test', ReplacementTest); + let r = this.owner.lookup('service:replacement-test'); + assert.strictEqual(r.myExample.message, 'Message from MyExample'); + r.myExample = { message: 'overridden' }; + assert.strictEqual(r.myExample.message, 'overridden'); + }); }); - }); ` }, }, From c17457e07db9e4afc66907ad0ba7877f05c77161 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 11 Mar 2025 14:05:38 -0400 Subject: [PATCH 21/23] fixing up assertion test --- .../-internals/metal/lib/injected_property.ts | 5 ++-- packages/@ember/service/tests/service_test.js | 23 +++++++++++++------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/injected_property.ts b/packages/@ember/-internals/metal/lib/injected_property.ts index 8382901df49..b6aebe800de 100644 --- a/packages/@ember/-internals/metal/lib/injected_property.ts +++ b/packages/@ember/-internals/metal/lib/injected_property.ts @@ -137,8 +137,9 @@ function inject2023(type: string, name: string | undefined, args: Parameters { // eslint-disable-next-line no-unused-vars class Foo extends EmberObject { @service main() {} } - }, /The @service decorator does not support method main/); - - assert.throws(() => { - @service - // eslint-disable-next-line no-unused-vars - class Foo extends EmberObject {} - }, /The @service decorator does not support class Foo/); + }, expectedAssertion); + + if (import.meta.env.VITE_STABLE_DECORATORS) { + assert.throws(() => { + @service + // eslint-disable-next-line no-unused-vars + class Foo extends EmberObject {} + }, /The @service decorator does not support class Foo/); + } } } ); From 80008d32556e8fabbbaf407b052790d1f3cfcad4 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 11 Mar 2025 14:23:36 -0400 Subject: [PATCH 22/23] don't try to test the assertions under legacy decorators --- packages/@ember/service/tests/service_test.js | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/@ember/service/tests/service_test.js b/packages/@ember/service/tests/service_test.js index 94114e2800a..b73a19175cf 100644 --- a/packages/@ember/service/tests/service_test.js +++ b/packages/@ember/service/tests/service_test.js @@ -130,24 +130,21 @@ moduleFor( ['@test throws when used in wrong syntactic position'](assert) { // I'm allowing the assertions to be different under the new decorator // standard because the assertions on the old one were pretty bad. - - let expectedAssertion = import.meta.env.VITE_STABLE_DECORATORS - ? /The @service decorator does not support method main/ - : /@computed can only be used on accessors or fields/; - - assert.throws(() => { - // eslint-disable-next-line no-unused-vars - class Foo extends EmberObject { - @service main() {} - } - }, expectedAssertion); - if (import.meta.env.VITE_STABLE_DECORATORS) { + assert.throws(() => { + // eslint-disable-next-line no-unused-vars + class Foo extends EmberObject { + @service main() {} + } + }, /The @service decorator does not support method main/); + assert.throws(() => { @service // eslint-disable-next-line no-unused-vars class Foo extends EmberObject {} }, /The @service decorator does not support class Foo/); + } else { + assert.expect(0); } } } From 42409ce43c3e511e54b2663c61193a6e06f624af Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 11 Mar 2025 14:24:52 -0400 Subject: [PATCH 23/23] keep using throw instead of assert --- packages/@ember/-internals/metal/lib/injected_property.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/injected_property.ts b/packages/@ember/-internals/metal/lib/injected_property.ts index b6aebe800de..8382901df49 100644 --- a/packages/@ember/-internals/metal/lib/injected_property.ts +++ b/packages/@ember/-internals/metal/lib/injected_property.ts @@ -137,9 +137,8 @@ function inject2023(type: string, name: string | undefined, args: Parameters