From b49f91dabe3738c939337689f8bb422c08727f24 Mon Sep 17 00:00:00 2001 From: Chris Garret Date: Mon, 28 Sep 2020 11:47:27 -0400 Subject: [PATCH 1/2] [BUGFIX beta] Generalize args proxy. Generalizes the args proxy for use in both components and helpers. This does change the semantics slightly of `positional` arguments for component managers, notably it would make it so using computed properties with them would require a bit more work from custom component managers. The new semantics are more inline with the future of Ember, but if there many custom managers that use positional args we may want to reconsider this change. Co-authored-by: Robert Jackson --- .../glimmer/lib/component-managers/custom.ts | 127 ++--------- .../glimmer/lib/utils/args-proxy.ts | 210 ++++++++++++++++++ .../custom-component-manager-test.js | 70 +++++- 3 files changed, 290 insertions(+), 117 deletions(-) create mode 100644 packages/@ember/-internals/glimmer/lib/utils/args-proxy.ts diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/custom.ts b/packages/@ember/-internals/glimmer/lib/component-managers/custom.ts index a93f6ff52fd..7f5f822a0d3 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/custom.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/custom.ts @@ -1,13 +1,9 @@ import { ENV } from '@ember/-internals/environment'; -import { CUSTOM_TAG_FOR } from '@ember/-internals/metal'; import { Factory } from '@ember/-internals/owner'; -import { HAS_NATIVE_PROXY } from '@ember/-internals/utils'; import { assert } from '@ember/debug'; -import { DEBUG } from '@glimmer/env'; import { + Arguments, Bounds, - CapturedArguments, - CapturedNamedArguments, ComponentCapabilities, ComponentDefinition, Destroyable, @@ -16,13 +12,13 @@ import { VMArguments, WithStaticLayout, } from '@glimmer/interfaces'; -import { createConstRef, Reference, valueForRef } from '@glimmer/reference'; -import { registerDestructor, reifyPositional } from '@glimmer/runtime'; +import { createConstRef, Reference } from '@glimmer/reference'; +import { registerDestructor } from '@glimmer/runtime'; import { unwrapTemplate } from '@glimmer/util'; -import { Tag, track } from '@glimmer/validator'; import { EmberVMEnvironment } from '../environment'; import RuntimeResolver from '../resolver'; import { OwnedTemplate } from '../template'; +import { argsProxyFor } from '../utils/args-proxy'; import AbstractComponentManager from './abstract'; const CAPABILITIES = { @@ -149,94 +145,6 @@ export interface ComponentArguments { named: Dict; } -function tagForNamedArg( - namedArgs: NamedArgs, - key: K -): Tag { - return track(() => valueForRef(namedArgs[key])); -} - -let namedArgsProxyFor: (namedArgs: CapturedNamedArguments, debugName?: string) => Args['named']; - -if (HAS_NATIVE_PROXY) { - namedArgsProxyFor = ( - namedArgs: NamedArgs, - debugName?: string - ) => { - let getTag = (key: keyof Args) => tagForNamedArg(namedArgs, key); - - let handler: ProxyHandler<{}> = { - get(_target, prop) { - let ref = namedArgs[prop as string]; - - if (ref !== undefined) { - return valueForRef(ref); - } else if (prop === CUSTOM_TAG_FOR) { - return getTag; - } - }, - - has(_target, prop) { - return namedArgs[prop as string] !== undefined; - }, - - ownKeys(_target) { - return Object.keys(namedArgs); - }, - - getOwnPropertyDescriptor(_target, prop) { - assert( - 'args proxies do not have real property descriptors, so you should never need to call getOwnPropertyDescriptor yourself. This code exists for enumerability, such as in for-in loops and Object.keys()', - namedArgs[prop as string] !== undefined - ); - - return { - enumerable: true, - configurable: true, - }; - }, - }; - - if (DEBUG) { - handler.set = function(_target, prop) { - assert( - `You attempted to set ${debugName}#${String( - prop - )} on a components arguments. Component arguments are immutable and cannot be updated directly, they always represent the values that are passed to your component. If you want to set default values, you should use a getter instead` - ); - - return false; - }; - } - - return new Proxy({}, handler); - }; -} else { - namedArgsProxyFor = (namedArgs: NamedArgs) => { - let getTag = (key: keyof Args) => tagForNamedArg(namedArgs, key); - - let proxy = {}; - - Object.defineProperty(proxy, CUSTOM_TAG_FOR, { - configurable: false, - enumerable: false, - value: getTag, - }); - - Object.keys(namedArgs).forEach(name => { - Object.defineProperty(proxy, name, { - enumerable: true, - configurable: true, - get() { - return valueForRef(namedArgs[name]); - }, - }); - }); - - return proxy; - }; -} - /** The CustomComponentManager allows addons to provide custom component implementations that integrate seamlessly into Ember. This is accomplished @@ -276,25 +184,20 @@ export default class CustomComponentManager create( env: EmberVMEnvironment, definition: CustomComponentDefinitionState, - args: VMArguments + vmArgs: VMArguments ): CustomComponentState { let { delegate } = definition; - let capturedArgs = args.capture(); - let { named, positional } = capturedArgs; - let namedArgsProxy = namedArgsProxyFor(named); + let args = argsProxyFor(vmArgs.capture(), 'component'); - let component = delegate.createComponent(definition.ComponentClass.class, { - named: namedArgsProxy, - positional: reifyPositional(positional), - }); + let component = delegate.createComponent(definition.ComponentClass.class, args); - let bucket = new CustomComponentState(delegate, component, capturedArgs, env, namedArgsProxy); + let bucket = new CustomComponentState(delegate, component, args, env); if (ENV._DEBUG_RENDER_TREE) { env.extra.debugRenderTree.create(bucket, { type: 'component', name: definition.name, - args: args.capture(), + args: vmArgs.capture(), instance: component, template: definition.template, }); @@ -317,12 +220,9 @@ export default class CustomComponentManager } if (hasUpdateHook(bucket.delegate)) { - let { delegate, component, args, namedArgsProxy } = bucket; + let { delegate, component, args } = bucket; - delegate.updateComponent(component, { - named: namedArgsProxy, - positional: reifyPositional(args.positional), - }); + delegate.updateComponent(component, args); } } @@ -383,9 +283,8 @@ export class CustomComponentState { constructor( public delegate: ManagerDelegate, public component: ComponentInstance, - public args: CapturedArguments, - public env: EmberVMEnvironment, - public namedArgsProxy: Args['named'] + public args: Arguments, + public env: EmberVMEnvironment ) { if (hasDestructors(delegate)) { registerDestructor(this, () => delegate.destroyComponent(component)); diff --git a/packages/@ember/-internals/glimmer/lib/utils/args-proxy.ts b/packages/@ember/-internals/glimmer/lib/utils/args-proxy.ts new file mode 100644 index 00000000000..a7ccf0559e5 --- /dev/null +++ b/packages/@ember/-internals/glimmer/lib/utils/args-proxy.ts @@ -0,0 +1,210 @@ +import { CUSTOM_TAG_FOR } from '@ember/-internals/metal'; +import { HAS_NATIVE_PROXY } from '@ember/-internals/utils'; +import { assert } from '@ember/debug'; +import { DEBUG } from '@glimmer/env'; +import { + Arguments, + CapturedArguments, + CapturedNamedArguments, + CapturedPositionalArguments, +} from '@glimmer/interfaces'; +import { Reference, valueForRef } from '@glimmer/reference'; +import { Tag, track } from '@glimmer/validator'; + +function convertToInt(prop: number | string | symbol): number | null { + if (typeof prop === 'symbol') return null; + + const num = Number(prop); + + if (isNaN(num)) return null; + + return num % 1 === 0 ? num : null; +} + +function tagForNamedArg(namedArgs: CapturedNamedArguments, key: string): Tag { + return track(() => { + if (key in namedArgs) { + valueForRef(namedArgs[key]); + } + }); +} + +function tagForPositionalArg(positionalArgs: CapturedPositionalArguments, key: string): Tag { + return track(() => { + if (key === '[]') { + // consume all of the tags in the positional array + positionalArgs.forEach(valueForRef); + } + + const parsed = convertToInt(key); + + if (parsed !== null && parsed < positionalArgs.length) { + // consume the tag of the referenced index + valueForRef(positionalArgs[parsed]); + } + }); +} + +export let argsProxyFor: ( + capturedArgs: CapturedArguments, + type: 'component' | 'helper' | 'modifier' +) => Arguments; + +if (HAS_NATIVE_PROXY) { + argsProxyFor = (capturedArgs, type) => { + const { named, positional } = capturedArgs; + + let getNamedTag = (key: string) => tagForNamedArg(named, key); + let getPositionalTag = (key: string) => tagForPositionalArg(positional, key); + + const namedHandler: ProxyHandler<{}> = { + get(_target, prop) { + const ref = named[prop as string]; + + if (ref !== undefined) { + return valueForRef(ref); + } else if (prop === CUSTOM_TAG_FOR) { + return getNamedTag; + } + }, + + has(_target, prop) { + return prop in named; + }, + + ownKeys(_target) { + return Object.keys(named); + }, + + isExtensible() { + return false; + }, + + getOwnPropertyDescriptor(_target, prop) { + assert( + 'args proxies do not have real property descriptors, so you should never need to call getOwnPropertyDescriptor yourself. This code exists for enumerability, such as in for-in loops and Object.keys()', + prop in named + ); + + return { + enumerable: true, + configurable: true, + }; + }, + }; + + const positionalHandler: ProxyHandler<[]> = { + get(target, prop) { + if (prop === 'length') { + return positional.length; + } + + const parsed = convertToInt(prop); + + if (parsed !== null && parsed < positional.length) { + return valueForRef(positional[parsed]); + } + + if (prop === CUSTOM_TAG_FOR) { + return getPositionalTag; + } + + return (target as any)[prop]; + }, + + isExtensible() { + return false; + }, + + has(_target, prop) { + const parsed = convertToInt(prop); + + return parsed !== null && parsed < positional.length; + }, + }; + + const namedTarget = Object.create(null); + const positionalTarget: unknown[] = []; + + if (DEBUG) { + const setHandler = function(_target: unknown, prop: symbol | string | number): never { + throw new Error( + `You attempted to set ${String( + prop + )} on the arguments of a component, helper, or modifier. Arguments are immutable and cannot be updated directly, they always represent the values that is passed down. If you want to set default values, you should use a getter and local tracked state instead.` + ); + }; + + const forInDebugHandler = (): never => { + throw new Error( + `Object.keys() was called on the positional arguments array for a ${type}, which is not supported. This function is a low-level function that should not need to be called for positional argument arrays. You may be attempting to iterate over the array using for...in instead of for...of.` + ); + }; + + namedHandler.set = setHandler; + positionalHandler.set = setHandler; + positionalHandler.ownKeys = forInDebugHandler; + } + + return { + named: new Proxy(namedTarget, namedHandler), + positional: new Proxy(positionalTarget, positionalHandler), + }; + }; +} else { + argsProxyFor = (capturedArgs, _type) => { + const { named, positional } = capturedArgs; + + let getNamedTag = (key: string) => tagForNamedArg(named, key); + let getPositionalTag = (key: string) => tagForPositionalArg(positional, key); + + let namedProxy = {}; + + Object.defineProperty(namedProxy, CUSTOM_TAG_FOR, { + configurable: false, + enumerable: false, + value: getNamedTag, + }); + + Object.keys(named).forEach(name => { + Object.defineProperty(namedProxy, name, { + enumerable: true, + configurable: true, + get() { + return valueForRef(named[name]); + }, + }); + }); + + let positionalProxy: unknown[] = []; + + Object.defineProperty(positionalProxy, CUSTOM_TAG_FOR, { + configurable: false, + enumerable: false, + value: getPositionalTag, + }); + + positional.forEach((ref: Reference, index: number) => { + Object.defineProperty(positionalProxy, index, { + enumerable: true, + configurable: true, + get() { + return valueForRef(ref); + }, + }); + }); + + if (DEBUG) { + // Prevent mutations in development mode. This will not prevent the + // proxy from updating, but will prevent assigning new values or pushing + // for instance. + Object.freeze(namedProxy); + Object.freeze(positionalProxy); + } + + return { + named: namedProxy, + positional: positionalProxy, + }; + }; +} diff --git a/packages/@ember/-internals/glimmer/tests/integration/custom-component-manager-test.js b/packages/@ember/-internals/glimmer/tests/integration/custom-component-manager-test.js index 0e7dbd68146..1463c7999b9 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/custom-component-manager-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/custom-component-manager-test.js @@ -304,7 +304,7 @@ moduleFor( let ComponentClass = setComponentManager( createBasicManager, EmberObject.extend({ - salutation: computed('args.positional', function() { + salutation: computed('args.positional.[]', function() { return this.args.positional[0] + ' ' + this.args.positional[1]; }), }) @@ -320,11 +320,11 @@ moduleFor( this.assertHTML(`

Yehuda Katz

`); } - ['@test positional params are updated if they change']() { + ['@test positional params are updated if they change (computed, arr tag)']() { let ComponentClass = setComponentManager( createBasicManager, EmberObject.extend({ - salutation: computed('args.positional', function() { + salutation: computed('args.positional.[]', function() { return this.args.positional[0] + ' ' + this.args.positional[1]; }), }) @@ -352,6 +352,70 @@ moduleFor( this.assertHTML(`

Chad Hietala

`); } + ['@test positional params are updated if they change (computed, individual tags)']() { + let ComponentClass = setComponentManager( + createBasicManager, + EmberObject.extend({ + salutation: computed('args.positional.0', 'args.positional.1', function() { + return this.args.positional[0] + ' ' + this.args.positional[1]; + }), + }) + ); + + this.registerComponent('foo-bar', { + template: `

{{salutation}}

`, + ComponentClass, + }); + + this.render('{{foo-bar firstName lastName}}', { + firstName: 'Yehuda', + lastName: 'Katz', + }); + + this.assertHTML(`

Yehuda Katz

`); + + runTask(() => + setProperties(this.context, { + firstName: 'Chad', + lastName: 'Hietala', + }) + ); + + this.assertHTML(`

Chad Hietala

`); + } + + ['@test positional params are updated if they change (native)']() { + let ComponentClass = setComponentManager( + createBasicManager, + class extends EmberObject { + get salutation() { + return this.args.positional[0] + ' ' + this.args.positional[1]; + } + } + ); + + this.registerComponent('foo-bar', { + template: `

{{salutation}}

`, + ComponentClass, + }); + + this.render('{{foo-bar firstName lastName}}', { + firstName: 'Yehuda', + lastName: 'Katz', + }); + + this.assertHTML(`

Yehuda Katz

`); + + runTask(() => + setProperties(this.context, { + firstName: 'Chad', + lastName: 'Hietala', + }) + ); + + this.assertHTML(`

Chad Hietala

`); + } + ['@test it can opt-in to running destructor'](assert) { let ComponentClass = setComponentManager( () => { From d7f56c44e4d5ec1d0e18449451419ecb045458ea Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 28 Sep 2020 14:01:00 -0400 Subject: [PATCH 2/2] [BUGFIX beta] Ensure custom modifiers use args proxy. Prior to this change, all custom modifiers **always** consumed every argument on install/update/destroy. This was because `reifyArgs` specifically reads all of them, and the actual usage was not consumption based. After this change, any modifiers using `modifierCapabilities('3.22')` will only consume the arguments that are actually used (named and positional). All modifiers that use `modifierCapabilities('3.13')` are unchanged (preserving backwards compatibility). --- .../glimmer/lib/modifiers/custom.ts | 112 ++-- .../angle-bracket-invocation-test.js | 26 +- .../custom-modifier-manager-test.js | 516 ++++++++++++------ 3 files changed, 431 insertions(+), 223 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts b/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts index 26803915ec5..f677d581525 100644 --- a/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts +++ b/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts @@ -1,10 +1,11 @@ import { Factory } from '@ember/-internals/owner'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; -import { CapturedArguments, Dict, ModifierManager, VMArguments } from '@glimmer/interfaces'; +import { Arguments, ModifierManager, VMArguments } from '@glimmer/interfaces'; import { registerDestructor, reifyArgs } from '@glimmer/runtime'; -import { createUpdatableTag, untrack } from '@glimmer/validator'; +import { createUpdatableTag, untrack, UpdatableTag } from '@glimmer/validator'; import { SimpleElement } from '@simple-dom/interface'; +import { argsProxyFor } from '../utils/args-proxy'; export interface CustomModifierDefinitionState { ModifierClass: Factory; @@ -13,21 +14,33 @@ export interface CustomModifierDefinitionState { } export interface OptionalCapabilities { - disableAutoTracking?: boolean; + '3.13': { + disableAutoTracking?: boolean; + }; + + // uses args proxy, does not provide a way to opt-out + '3.22': { + disableAutoTracking?: boolean; + }; } export interface Capabilities { disableAutoTracking: boolean; + useArgsProxy: boolean; } -export function capabilities( - managerAPI: string, - optionalFeatures: OptionalCapabilities = {} +export function capabilities( + managerAPI: Version, + optionalFeatures: OptionalCapabilities[Version] = {} ): Capabilities { - assert('Invalid modifier manager compatibility specified', managerAPI === '3.13'); + assert( + 'Invalid modifier manager compatibility specified', + managerAPI === '3.13' || managerAPI === '3.22' + ); return { disableAutoTracking: Boolean(optionalFeatures.disableAutoTracking), + useArgsProxy: managerAPI === '3.13' ? false : true, }; } @@ -53,32 +66,21 @@ export class CustomModifierDefinition { } } -export class CustomModifierState { - public tag = createUpdatableTag(); +export interface CustomModifierState { + tag: UpdatableTag; + element: SimpleElement; + delegate: ModifierManagerDelegate; + modifier: ModifierInstance; + args: Arguments; debugName?: string; - - constructor( - public element: SimpleElement, - public delegate: ModifierManagerDelegate, - public modifier: ModifierInstance, - public args: CapturedArguments - ) { - registerDestructor(this, () => delegate.destroyModifier(modifier, reifyArgs(args))); - } -} - -// TODO: export ICapturedArgumentsValue from glimmer and replace this -export interface Args { - named: Dict; - positional: unknown[]; } export interface ModifierManagerDelegate { capabilities: Capabilities; - createModifier(factory: unknown, args: Args): ModifierInstance; - installModifier(instance: ModifierInstance, element: SimpleElement, args: Args): void; - updateModifier(instance: ModifierInstance, args: Args): void; - destroyModifier(instance: ModifierInstance, args: Args): void; + createModifier(factory: unknown, args: Arguments): ModifierInstance; + installModifier(instance: ModifierInstance, element: SimpleElement, args: Arguments): void; + updateModifier(instance: ModifierInstance, args: Arguments): void; + destroyModifier(instance: ModifierInstance, args: Arguments): void; } /** @@ -114,18 +116,49 @@ class InteractiveCustomModifierManager create( element: SimpleElement, definition: CustomModifierDefinitionState, - args: VMArguments + vmArgs: VMArguments ) { let { delegate, ModifierClass } = definition; - const capturedArgs = args.capture(); + let capturedArgs = vmArgs.capture(); + + assert( + 'Custom modifier managers must define their capabilities using the capabilities() helper function', + typeof delegate.capabilities === 'object' && delegate.capabilities !== null + ); - let instance = definition.delegate.createModifier(ModifierClass, reifyArgs(capturedArgs)); - let state = new CustomModifierState(element, delegate, instance, capturedArgs); + let useArgsProxy = delegate.capabilities.useArgsProxy; + + let args = useArgsProxy ? argsProxyFor(capturedArgs, 'modifier') : reifyArgs(capturedArgs); + let instance = delegate.createModifier(ModifierClass, args); + + let tag = createUpdatableTag(); + let state: CustomModifierState; + if (useArgsProxy) { + state = { + tag, + element, + delegate, + args, + modifier: instance, + }; + } else { + state = { + tag, + element, + delegate, + modifier: instance, + get args() { + return reifyArgs(capturedArgs); + }, + }; + } if (DEBUG) { state.debugName = definition.name; } + registerDestructor(state, () => delegate.destroyModifier(instance, state.args)); + return state; } @@ -140,30 +173,23 @@ class InteractiveCustomModifierManager install(state: CustomModifierState) { let { element, args, delegate, modifier } = state; - assert( - 'Custom modifier managers must define their capabilities using the capabilities() helper function', - typeof delegate.capabilities === 'object' && delegate.capabilities !== null - ); - let { capabilities } = delegate; - let argsValue = reifyArgs(args); if (capabilities.disableAutoTracking === true) { - untrack(() => delegate.installModifier(modifier, element, argsValue)); + untrack(() => delegate.installModifier(modifier, element, args)); } else { - delegate.installModifier(modifier, element, argsValue); + delegate.installModifier(modifier, element, args); } } update(state: CustomModifierState) { let { args, delegate, modifier } = state; let { capabilities } = delegate; - let argsValue = reifyArgs(args); if (capabilities.disableAutoTracking === true) { - untrack(() => delegate.updateModifier(modifier, argsValue)); + untrack(() => delegate.updateModifier(modifier, args)); } else { - delegate.updateModifier(modifier, argsValue); + delegate.updateModifier(modifier, args); } } diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/angle-bracket-invocation-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/angle-bracket-invocation-test.js index 40cf897a6c8..8a7139385b0 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/angle-bracket-invocation-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/angle-bracket-invocation-test.js @@ -1258,6 +1258,15 @@ moduleFor( moduleFor( 'Element modifiers on AngleBracket components', class extends RenderingTestCase { + assertNamedArgs(actual, expected, message) { + // `actual` is likely to be a named args proxy, while the `deepEqual` below would see + // the values as the same it would still flag as not deep equals because the constructors + // of the two objects do not match (one is a proxy, one is Object) + let reifiedActual = Object.assign({}, actual); + + this.assert.deepEqual(reifiedActual, expected, message); + } + '@test modifiers are forwarded to a single element receiving the splattributes'(assert) { let modifierParams = null; let modifierNamedArgs = null; @@ -1277,8 +1286,8 @@ moduleFor( }) ); this.render('', {}); - assert.deepEqual(modifierParams, ['something']); - assert.deepEqual(modifierNamedArgs, { foo: 'else' }); + assert.deepEqual(modifierParams, ['something'], 'positional arguments'); + this.assertNamedArgs(modifierNamedArgs, { foo: 'else' }, 'named arguments'); assert.equal( modifiedElement && modifiedElement.getAttribute('id'), 'inner-div', @@ -1293,12 +1302,13 @@ moduleFor( template: '
Foo
Bar
', }); + let test = this; this.registerModifier( 'bar', BaseModifier.extend({ didInsertElement(params, namedArgs) { assert.deepEqual(params, ['something']); - assert.deepEqual(namedArgs, { foo: 'else' }); + test.assertNamedArgs(namedArgs, { foo: 'else' }); if (this.element) { elementIds.push(this.element.getAttribute('id')); } @@ -1341,7 +1351,7 @@ moduleFor( foo: 'else', }); assert.deepEqual(modifierParams, ['something']); - assert.deepEqual(modifierNamedArgs, { foo: 'else' }); + this.assertNamedArgs(modifierNamedArgs, { foo: 'else' }); assert.equal( modifiedElement && modifiedElement.getAttribute('id'), 'inner-div', @@ -1349,7 +1359,7 @@ moduleFor( ); runTask(() => setProperties(this.context, { something: 'another', foo: 'thingy' })); assert.deepEqual(modifierParams, ['another']); - assert.deepEqual(modifierNamedArgs, { foo: 'thingy' }); + this.assertNamedArgs(modifierNamedArgs, { foo: 'thingy' }); assert.equal( modifiedElement && modifiedElement.getAttribute('id'), 'inner-div', @@ -1435,7 +1445,7 @@ moduleFor( { foo: 'bar' } ); assert.deepEqual(modifierParams, ['bar']); - assert.deepEqual(modifierNamedArgs, { foo: 'bar' }); + this.assertNamedArgs(modifierNamedArgs, { foo: 'bar' }); assert.equal( modifiedElement && modifiedElement.getAttribute('id'), 'inner-div', @@ -1443,7 +1453,7 @@ moduleFor( ); runTask(() => setProperties(this.context, { foo: 'qux' })); assert.deepEqual(modifierParams, ['qux']); - assert.deepEqual(modifierNamedArgs, { foo: 'qux' }); + this.assertNamedArgs(modifierNamedArgs, { foo: 'qux' }); assert.equal( modifiedElement && modifiedElement.getAttribute('id'), 'inner-div', @@ -1486,7 +1496,7 @@ moduleFor( { foo: 'bar' } ); assert.deepEqual(modifierParams, ['bar']); - assert.deepEqual(modifierNamedArgs, { foo: 'bar' }); + this.assertNamedArgs(modifierNamedArgs, { foo: 'bar' }); assert.deepEqual( elementIds, ['outer-div', 'inner-div'], diff --git a/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js b/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js index 503878e65d6..3565fdc6f51 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js @@ -5,11 +5,8 @@ import { setModifierManager, modifierCapabilities } from '@ember/-internals/glim import { set, tracked } from '@ember/-internals/metal'; import { backtrackingMessageFor } from '../utils/backtracking-rerender'; -class ModifierManagerTest extends RenderingTestCase {} - class CustomModifierManager { constructor(owner) { - this.capabilities = modifierCapabilities('3.13'); this.owner = owner; } @@ -32,100 +29,283 @@ class CustomModifierManager { instance.willDestroyElement(); } } +class ModifierManagerTest extends RenderingTestCase { + '@test throws a useful error when missing capabilities'() { + this.registerModifier( + 'foo-bar', + setModifierManager(() => { + return { + createModifier() {}, + installModifier() {}, + updateModifier() {}, + destroyModifier() {}, + }; + }, {}) + ); + + expectAssertion(() => { + this.render('

hello world

'); + }, /Custom modifier managers must define their capabilities/); + } -moduleFor( - 'Basic Custom Modifier Manager', - class extends ModifierManagerTest { - '@test throws a useful error when missing capabilities'() { - this.registerModifier( - 'foo-bar', - setModifierManager(() => { - return { - createModifier() {}, - installModifier() {}, - updateModifier() {}, - destroyModifier() {}, - }; - }, {}) - ); + '@test can register a custom element modifier and render it'(assert) { + let ModifierClass = setModifierManager( + owner => { + return new this.CustomModifierManager(owner); + }, + EmberObject.extend({ + didInsertElement() {}, + didUpdate() {}, + willDestroyElement() {}, + }) + ); + + this.registerModifier( + 'foo-bar', + ModifierClass.extend({ + didInsertElement() { + assert.ok(true, 'Called didInsertElement'); + }, + }) + ); - expectAssertion(() => { - this.render('

hello world

'); - }, /Custom modifier managers must define their capabilities/); - } + this.render('

hello world

'); + this.assertHTML(`

hello world

`); + } - '@test can register a custom element modifier and render it'(assert) { - let ModifierClass = setModifierManager( - owner => { - return new CustomModifierManager(owner); + '@test custom lifecycle hooks'(assert) { + assert.expect(9); + let ModifierClass = setModifierManager( + owner => { + return new this.CustomModifierManager(owner); + }, + EmberObject.extend({ + didInsertElement() {}, + didUpdate() {}, + willDestroyElement() {}, + }) + ); + + this.registerModifier( + 'foo-bar', + ModifierClass.extend({ + didUpdate([truthy]) { + assert.ok(true, 'Called didUpdate'); + assert.equal(truthy, 'true', 'gets updated args'); }, - EmberObject.extend({ - didInsertElement() {}, - didUpdate() {}, - willDestroyElement() {}, - }) - ); + didInsertElement([truthy]) { + assert.ok(true, 'Called didInsertElement'); + assert.equal(truthy, true, 'gets initial args'); + }, + willDestroyElement() { + assert.ok(true, 'Called willDestroyElement'); + }, + }) + ); - this.registerModifier( - 'foo-bar', - ModifierClass.extend({ - didInsertElement() { - assert.ok(true, 'Called didInsertElement'); - }, - }) - ); + this.render('{{#if truthy}}

hello world

{{/if}}', { + truthy: true, + }); + this.assertHTML(`

hello world

`); - this.render('

hello world

'); - this.assertHTML(`

hello world

`); - } + runTask(() => set(this.context, 'truthy', 'true')); - '@test custom lifecycle hooks'(assert) { - assert.expect(9); - let ModifierClass = setModifierManager( - owner => { - return new CustomModifierManager(owner); + runTask(() => set(this.context, 'truthy', false)); + + runTask(() => set(this.context, 'truthy', true)); + } + + '@test associates manager even through an inheritance structure'(assert) { + assert.expect(5); + let ModifierClass = setModifierManager( + owner => { + return new this.CustomModifierManager(owner); + }, + EmberObject.extend({ + didInsertElement() {}, + didUpdate() {}, + willDestroyElement() {}, + }) + ); + + ModifierClass = ModifierClass.extend({ + didInsertElement([truthy]) { + this._super(...arguments); + assert.ok(true, 'Called didInsertElement'); + assert.equal(truthy, true, 'gets initial args'); + }, + }); + + this.registerModifier( + 'foo-bar', + ModifierClass.extend({ + didInsertElement([truthy]) { + this._super(...arguments); + assert.ok(true, 'Called didInsertElement'); + assert.equal(truthy, true, 'gets initial args'); }, - EmberObject.extend({ - didInsertElement() {}, - didUpdate() {}, - willDestroyElement() {}, - }) - ); + }) + ); - this.registerModifier( - 'foo-bar', - ModifierClass.extend({ - didUpdate([truthy]) { - assert.ok(true, 'Called didUpdate'); - assert.equal(truthy, 'true', 'gets updated args'); - }, - didInsertElement([truthy]) { - assert.ok(true, 'Called didInsertElement'); - assert.equal(truthy, true, 'gets initial args'); - }, - willDestroyElement() { - assert.ok(true, 'Called willDestroyElement'); - }, - }) - ); + this.render('

hello world

', { + truthy: true, + }); + this.assertHTML(`

hello world

`); + } - this.render('{{#if truthy}}

hello world

{{/if}}', { - truthy: true, - }); - this.assertHTML(`

hello world

`); + '@test can give consistent access to underlying DOM element'(assert) { + assert.expect(4); + let ModifierClass = setModifierManager( + owner => { + return new this.CustomModifierManager(owner); + }, + EmberObject.extend({ + didInsertElement() {}, + didUpdate() {}, + willDestroyElement() {}, + }) + ); + + this.registerModifier( + 'foo-bar', + ModifierClass.extend({ + savedElement: undefined, + didInsertElement(positional) { + // consume first positional argument (ensures updates run) + positional[0]; + + assert.equal(this.element.tagName, 'H1'); + this.set('savedElement', this.element); + }, + didUpdate() { + assert.equal(this.element, this.savedElement); + }, + willDestroyElement() { + assert.equal(this.element, this.savedElement); + }, + }) + ); + + this.render('

hello world

', { + truthy: true, + }); + this.assertHTML(`

hello world

`); + + runTask(() => set(this.context, 'truthy', 'true')); + } + + '@test lifecycle hooks are autotracked by default'(assert) { + let TrackedClass = EmberObject.extend({ + count: tracked({ value: 0 }), + }); + + let trackedOne = TrackedClass.create(); + let trackedTwo = TrackedClass.create(); + + let insertCount = 0; + let updateCount = 0; + + let ModifierClass = setModifierManager( + owner => { + return new this.CustomModifierManager(owner); + }, + EmberObject.extend({ + didInsertElement() {}, + didUpdate() {}, + willDestroyElement() {}, + }) + ); + + this.registerModifier( + 'foo-bar', + ModifierClass.extend({ + didInsertElement() { + // track the count of the first item + trackedOne.count; + insertCount++; + }, + + didUpdate() { + // track the count of the second item + trackedTwo.count; + updateCount++; + }, + }) + ); + + this.render('

hello world

'); + this.assertHTML(`

hello world

`); + + assert.equal(insertCount, 1); + assert.equal(updateCount, 0); + + runTask(() => trackedTwo.count++); + assert.equal(updateCount, 0); - runTask(() => set(this.context, 'truthy', 'true')); + runTask(() => trackedOne.count++); + assert.equal(updateCount, 1); - runTask(() => set(this.context, 'truthy', false)); + runTask(() => trackedOne.count++); + assert.equal(updateCount, 1); - runTask(() => set(this.context, 'truthy', true)); + runTask(() => trackedTwo.count++); + assert.equal(updateCount, 2); + } + + '@test provides a helpful assertion when mutating a value that was consumed already'() { + class Person { + @tracked name = 'bob'; } - '@test associates manager even through an inheritance structure'(assert) { - assert.expect(5); + let ModifierClass = setModifierManager( + owner => { + return new this.CustomModifierManager(owner); + }, + class { + static create() { + return new this(); + } + + didInsertElement() {} + didUpdate() {} + willDestroyElement() {} + } + ); + + this.registerModifier( + 'foo-bar', + class MyModifier extends ModifierClass { + didInsertElement([person]) { + person.name; + person.name = 'sam'; + } + } + ); + + let expectedMessage = backtrackingMessageFor('name', 'Person', { + renderTree: ['\\(instance of a `foo-bar` modifier\\)'], + }); + + expectAssertion(() => { + this.render('

hello world

', { person: new Person() }); + }, expectedMessage); + } +} + +moduleFor( + 'Basic Custom Modifier Manager: 3.13', + class extends ModifierManagerTest { + CustomModifierManager = class extends CustomModifierManager { + capabilities = modifierCapabilities('3.13'); + }; + + '@test modifers consume all arguments'(assert) { + let insertCount = 0; + let updateCount = 0; + let ModifierClass = setModifierManager( owner => { - return new CustomModifierManager(owner); + return new this.CustomModifierManager(owner); }, EmberObject.extend({ didInsertElement() {}, @@ -134,36 +314,58 @@ moduleFor( }) ); - ModifierClass = ModifierClass.extend({ - didInsertElement([truthy]) { - this._super(...arguments); - assert.ok(true, 'Called didInsertElement'); - assert.equal(truthy, true, 'gets initial args'); - }, - }); - this.registerModifier( 'foo-bar', ModifierClass.extend({ - didInsertElement([truthy]) { - this._super(...arguments); - assert.ok(true, 'Called didInsertElement'); - assert.equal(truthy, true, 'gets initial args'); + didInsertElement(_positional, named) { + insertCount++; + + // consume qux + named.qux; + }, + + didUpdate(_positiona, named) { + updateCount++; + + // consume qux + named.qux; }, }) ); - this.render('

hello world

', { - truthy: true, + this.render('

hello world

', { + bar: 'bar', + qux: 'quz', }); + this.assertHTML(`

hello world

`); + + assert.equal(insertCount, 1); + assert.equal(updateCount, 0); + + runTask(() => set(this.context, 'bar', 'other bar')); + assert.equal(updateCount, 1); + + runTask(() => set(this.context, 'qux', 'quuuuxxxxxx')); + assert.equal(updateCount, 2); } + } +); + +moduleFor( + 'Basic Custom Modifier Manager: 3.22', + class extends ModifierManagerTest { + CustomModifierManager = class extends CustomModifierManager { + capabilities = modifierCapabilities('3.22'); + }; + + '@test modifers only track positional arguments they consume'(assert) { + let insertCount = 0; + let updateCount = 0; - '@test can give consistent access to underlying DOM element'(assert) { - assert.expect(4); let ModifierClass = setModifierManager( owner => { - return new CustomModifierManager(owner); + return new this.CustomModifierManager(owner); }, EmberObject.extend({ didInsertElement() {}, @@ -175,42 +377,51 @@ moduleFor( this.registerModifier( 'foo-bar', ModifierClass.extend({ - savedElement: undefined, - didInsertElement() { - assert.equal(this.element.tagName, 'H1'); - this.set('savedElement', this.element); - }, - didUpdate() { - assert.equal(this.element, this.savedElement); + didInsertElement(positional) { + insertCount++; + + // consume the second positional + positional[1]; }, - willDestroyElement() { - assert.equal(this.element, this.savedElement); + + didUpdate(positional) { + updateCount++; + + // consume the second positional + positional[1]; }, }) ); - this.render('

hello world

', { - truthy: true, - }); + this.render( + '

hello world

', + { + positionOne: 'first!!!', + positionTwo: 'second :(', + bar: 'bar', + qux: 'quz', + } + ); + this.assertHTML(`

hello world

`); - runTask(() => set(this.context, 'truthy', 'true')); - } + assert.equal(insertCount, 1); + assert.equal(updateCount, 0); - '@test lifecycle hooks are autotracked by default'(assert) { - let TrackedClass = EmberObject.extend({ - count: tracked({ value: 0 }), - }); + runTask(() => set(this.context, 'positionOne', 'no first?')); + assert.equal(updateCount, 0); - let trackedOne = TrackedClass.create(); - let trackedTwo = TrackedClass.create(); + runTask(() => set(this.context, 'positionTwo', 'YASSSSSSS!!!')); + assert.equal(updateCount, 1); + } + '@test modifers only track named arguments they consume'(assert) { let insertCount = 0; let updateCount = 0; let ModifierClass = setModifierManager( owner => { - return new CustomModifierManager(owner); + return new this.CustomModifierManager(owner); }, EmberObject.extend({ didInsertElement() {}, @@ -222,76 +433,37 @@ moduleFor( this.registerModifier( 'foo-bar', ModifierClass.extend({ - didInsertElement() { - // track the count of the first item - trackedOne.count; + didInsertElement(_positional, named) { insertCount++; + + // consume qux + named.qux; }, - didUpdate() { - // track the count of the second item - trackedTwo.count; + didUpdate(_positiona, named) { updateCount++; + + // consume qux + named.qux; }, }) ); - this.render('

hello world

'); + this.render('

hello world

', { + bar: 'bar', + qux: 'quz', + }); + this.assertHTML(`

hello world

`); assert.equal(insertCount, 1); assert.equal(updateCount, 0); - runTask(() => trackedTwo.count++); + runTask(() => set(this.context, 'bar', 'other bar')); assert.equal(updateCount, 0); - runTask(() => trackedOne.count++); + runTask(() => set(this.context, 'qux', 'quuuuxxxxxx')); assert.equal(updateCount, 1); - - runTask(() => trackedOne.count++); - assert.equal(updateCount, 1); - - runTask(() => trackedTwo.count++); - assert.equal(updateCount, 2); - } - - '@test provides a helpful assertion when mutating a value that was consumed already'() { - class Person { - @tracked name = 'bob'; - } - - let ModifierClass = setModifierManager( - owner => { - return new CustomModifierManager(owner); - }, - class { - static create() { - return new this(); - } - - didInsertElement() {} - didUpdate() {} - willDestroyElement() {} - } - ); - - this.registerModifier( - 'foo-bar', - class MyModifier extends ModifierClass { - didInsertElement([person]) { - person.name; - person.name = 'sam'; - } - } - ); - - let expectedMessage = backtrackingMessageFor('name', 'Person', { - renderTree: ['\\(instance of a `foo-bar` modifier\\)'], - }); - - expectAssertion(() => { - this.render('

hello world

', { person: new Person() }); - }, expectedMessage); } } );