From 1acd6d2c24a55b1376e8d46472d1a3c83b637004 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Tue, 29 Sep 2020 18:59:05 -0400 Subject: [PATCH] [BUGFIX beta] Make modifier manager 3.22 accept the resolved value directly. When using `modifierCapabilities('3.13')` (the only allowed value in all released Ember versions) you receive the result of `owner.factoryFor` directly. If you want the raw value, you have to grab it off of the first argument to `createModifier` like: ```js class CustomModifierManager { capabilities = modifierCapabilities('3.13'); constructor(owner) { this.owner = owner; } createModifier(factory, args) { let Modifier = factory.class; return new Modifier(owner, args) } // ...snip... } ``` In order to align the modifier world with the way helpers and component managers work, this commit changes to passing the `owner.factoryFor(...).class` directly to `createModifier` instead. So the above snippet would be rewritten to: ```js class CustomModifierManager { capabilities = modifierCapabilities('3.22'); constructor(owner) { this.owner = owner; } createModifier(Modifier, args) { return new Modifier(owner, args) } // ...snip... } ``` --- .../glimmer/lib/modifiers/custom.ts | 10 +- .../custom-modifier-manager-test.js | 159 +++++++++++++++--- 2 files changed, 140 insertions(+), 29 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts b/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts index f677d581525..3dda936be71 100644 --- a/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts +++ b/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts @@ -18,6 +18,7 @@ export interface OptionalCapabilities { disableAutoTracking?: boolean; }; + // passes factoryFor(...).class to `.createModifier` // uses args proxy, does not provide a way to opt-out '3.22': { disableAutoTracking?: boolean; @@ -27,6 +28,7 @@ export interface OptionalCapabilities { export interface Capabilities { disableAutoTracking: boolean; useArgsProxy: boolean; + passFactoryToCreate: boolean; } export function capabilities( @@ -41,6 +43,7 @@ export function capabilities( return { disableAutoTracking: Boolean(optionalFeatures.disableAutoTracking), useArgsProxy: managerAPI === '3.13' ? false : true, + passFactoryToCreate: managerAPI === '3.13', }; } @@ -126,10 +129,13 @@ class InteractiveCustomModifierManager typeof delegate.capabilities === 'object' && delegate.capabilities !== null ); - let useArgsProxy = delegate.capabilities.useArgsProxy; + let { useArgsProxy, passFactoryToCreate } = delegate.capabilities; let args = useArgsProxy ? argsProxyFor(capturedArgs, 'modifier') : reifyArgs(capturedArgs); - let instance = delegate.createModifier(ModifierClass, args); + let instance = delegate.createModifier( + passFactoryToCreate ? ModifierClass : ModifierClass.class, + args + ); let tag = createUpdatableTag(); let state: CustomModifierState; 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 3565fdc6f51..bd47728d4f4 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,30 +5,6 @@ import { setModifierManager, modifierCapabilities } from '@ember/-internals/glim import { set, tracked } from '@ember/-internals/metal'; import { backtrackingMessageFor } from '../utils/backtracking-rerender'; -class CustomModifierManager { - constructor(owner) { - this.owner = owner; - } - - createModifier(factory, args) { - return factory.create(args); - } - - installModifier(instance, element, args) { - instance.element = element; - let { positional, named } = args; - instance.didInsertElement(positional, named); - } - - updateModifier(instance, args) { - let { positional, named } = args; - instance.didUpdate(positional, named); - } - - destroyModifier(instance) { - instance.willDestroyElement(); - } -} class ModifierManagerTest extends RenderingTestCase { '@test throws a useful error when missing capabilities'() { this.registerModifier( @@ -295,8 +271,32 @@ class ModifierManagerTest extends RenderingTestCase { moduleFor( 'Basic Custom Modifier Manager: 3.13', class extends ModifierManagerTest { - CustomModifierManager = class extends CustomModifierManager { + CustomModifierManager = class CustomModifierManager { capabilities = modifierCapabilities('3.13'); + + constructor(owner) { + this.owner = owner; + } + + createModifier(factory, args) { + // factory is the owner.factoryFor result + return factory.create(args); + } + + installModifier(instance, element, args) { + instance.element = element; + let { positional, named } = args; + instance.didInsertElement(positional, named); + } + + updateModifier(instance, args) { + let { positional, named } = args; + instance.didUpdate(positional, named); + } + + destroyModifier(instance) { + instance.willDestroyElement(); + } }; '@test modifers consume all arguments'(assert) { @@ -355,8 +355,31 @@ moduleFor( moduleFor( 'Basic Custom Modifier Manager: 3.22', class extends ModifierManagerTest { - CustomModifierManager = class extends CustomModifierManager { + CustomModifierManager = class CustomModifierManager { capabilities = modifierCapabilities('3.22'); + + constructor(owner) { + this.owner = owner; + } + + createModifier(Modifier, args) { + return Modifier.create(args); + } + + installModifier(instance, element, args) { + instance.element = element; + let { positional, named } = args; + instance.didInsertElement(positional, named); + } + + updateModifier(instance, args) { + let { positional, named } = args; + instance.didUpdate(positional, named); + } + + destroyModifier(instance) { + instance.willDestroyElement(); + } }; '@test modifers only track positional arguments they consume'(assert) { @@ -475,7 +498,89 @@ moduleFor( return { isInteractive: false }; } - [`@test doesn't trigger lifecycle hooks when non-interactive`](assert) { + [`@test doesn't trigger lifecycle hooks when non-interactive: modifierCapabilities('3.13')`]( + assert + ) { + class CustomModifierManager { + capabilities = modifierCapabilities('3.13'); + + constructor(owner) { + this.owner = owner; + } + + createModifier(factory, args) { + return factory.create(args); + } + + installModifier(instance, element, args) { + instance.element = element; + let { positional, named } = args; + instance.didInsertElement(positional, named); + } + + updateModifier(instance, args) { + let { positional, named } = args; + instance.didUpdate(positional, named); + } + + destroyModifier(instance) { + instance.willDestroyElement(); + } + } + let ModifierClass = setModifierManager( + owner => { + return new CustomModifierManager(owner); + }, + EmberObject.extend({ + didInsertElement() { + assert.ok(false); + }, + didUpdate() { + assert.ok(false); + }, + willDestroyElement() { + assert.ok(false); + }, + }) + ); + + this.registerModifier('foo-bar', ModifierClass); + + this.render('

hello world

'); + runTask(() => this.context.set('baz', 'Hello')); + + this.assertHTML('

hello world

'); + } + + [`@test doesn't trigger lifecycle hooks when non-interactive: modifierCapabilities('3.22')`]( + assert + ) { + class CustomModifierManager { + capabilities = modifierCapabilities('3.22'); + + constructor(owner) { + this.owner = owner; + } + + createModifier(Modifier, args) { + return Modifier.create(args); + } + + installModifier(instance, element, args) { + instance.element = element; + let { positional, named } = args; + instance.didInsertElement(positional, named); + } + + updateModifier(instance, args) { + let { positional, named } = args; + instance.didUpdate(positional, named); + } + + destroyModifier(instance) { + instance.willDestroyElement(); + } + } let ModifierClass = setModifierManager( owner => { return new CustomModifierManager(owner);