feat(modifier-managers): use 3.22 capabilities

BREAKING CHANGE: destruction behavior changed to use new args proxy from 3.22 Resolves the issue described by: emberjs/ember.js#19162 fixed by: emberjs/ember.js#19163
NullVoxPopuli · Oct 18, 2020 · ebd5e12 · ebd5e12
1 parent a8d4da7
commit ebd5e12
Show file tree

Hide file tree

Showing 7 changed files with 891 additions and 52 deletions.
diff --git a/addon/-private/class/modifier-manager.ts b/addon/-private/class/modifier-manager.ts
@@ -1,6 +1,8 @@
 import { capabilities } from '@ember/modifier';
+import { gte } from 'ember-compatibility-helpers';
 import { set } from '@ember/object';
 import { destroy, registerDestructor } from '@ember/destroyable';
+import { assert } from '@ember/debug';
 
 import ClassBasedModifier from './modifier';
 import { ModifierArgs } from 'ember-modifier/-private/interfaces';
@@ -10,16 +12,40 @@ function destroyModifier(modifier: ClassBasedModifier): void {
   modifier.willDestroy();
 }
 
+type CreateModifierArgs313 = [
+  { owner: unknown; class: typeof ClassBasedModifier },
+  ModifierArgs
+];
+type CreateModifierArgs322 = [typeof ClassBasedModifier, ModifierArgs];
+type CreateModifierArgs = CreateModifierArgs313 | CreateModifierArgs322;
+
 export default class ClassBasedModifierManager {
-  capabilities = capabilities('3.13');
+  capabilities = capabilities(gte('3.22.0') ? '3.22' : '3.13');
 
   constructor(private owner: unknown) {}
 
   createModifier(
-    factory: { owner: unknown; class: typeof ClassBasedModifier },
-    args: ModifierArgs
+    ...createModifierArgs: CreateModifierArgs
   ): ClassBasedModifier {
-    const Modifier = factory.class;
+    const [factoryOrClass, args] = createModifierArgs;
+
+    let Modifier: typeof ClassBasedModifier;
+
+    if (gte('3.22.0')) {
+      assert(
+        'expected factory to be a class reference',
+        !('class' in factoryOrClass)
+      );
+
+      Modifier = factoryOrClass;
+    } else {
+      assert(
+        'createModifier expected an object with keys owner and class',
+        'class' in factoryOrClass
+      );
+
+      Modifier = factoryOrClass.class;
+    }
 
     const modifier = new Modifier(this.owner, args);
 
@@ -35,7 +61,6 @@ export default class ClassBasedModifierManager {
   }
 
   updateModifier(instance: ClassBasedModifier, args: ModifierArgs): void {
-    // TODO: this should be an args proxy
     set(instance, 'args', args);
     instance.didUpdateArguments();
     instance.didReceiveArguments();

diff --git a/addon/-private/functional/modifier-manager.ts b/addon/-private/functional/modifier-manager.ts
@@ -1,4 +1,5 @@
 import { capabilities } from '@ember/modifier';
+import { gte } from 'ember-compatibility-helpers';
 import { FunctionalModifier } from './modifier';
 import { ModifierArgs } from '../interfaces';
 
@@ -28,15 +29,25 @@ function setup(
   MODIFIER_TEARDOWNS.set(modifier, teardown);
 }
 
+type CreateModifierArgs313 = [Factory];
+type CreateModifierArgs322 = [FunctionalModifier];
+type CreateModifierArgs = CreateModifierArgs313 | CreateModifierArgs322;
+
 class FunctionalModifierManager {
-  capabilities = capabilities('3.13');
+  capabilities = capabilities(gte('3.22.0') ? '3.22' : '3.13');
 
-  createModifier(factory: Factory): FunctionalModifier {
+  createModifier(...[factoryOrClass]: CreateModifierArgs): FunctionalModifier {
     // This looks superfluous, but this is creating a new instance
     // of a function -- this is important so that each instance of the
     // created modifier can have its own state which is stored in
     // the MODIFIER_ELEMENTS and MODIFIER_TEARDOWNS WeakMaps
-    return (...args) => factory.class(...args);
+    return (...args) => {
+      if ('class' in factoryOrClass) {
+        return factoryOrClass.class(...args);
+      }
+
+      return factoryOrClass(...args);
+    };
   }
 
   installModifier(

diff --git a/package.json b/package.json
@@ -41,6 +41,7 @@
     "ember-cli-normalize-entity-name": "^1.0.0",
     "ember-cli-string-utils": "^1.1.0",
     "ember-cli-typescript": "^3.1.3",
+    "ember-compatibility-helpers": "^1.2.1",
     "ember-destroyable-polyfill": "^2.0.2",
     "ember-modifier-manager-polyfill": "^1.2.0"
   },
@@ -73,7 +74,7 @@
     "ember-maybe-import-regenerator": "^0.1.6",
     "ember-qunit": "^4.6.0",
     "ember-resolver": "^8.0.0",
-    "ember-source": "~3.20.2",
+    "ember-source": "~3.22.0",
     "ember-source-channel-url": "^2.0.1",
     "ember-template-lint": "^2.9.1",
     "ember-try": "^1.4.0",

diff --git a/tests/integration/modifiers/class-modifier-test.ts b/tests/integration/modifiers/class-modifier-test.ts
@@ -1,3 +1,4 @@
+import { gte } from 'ember-compatibility-helpers';
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
 import { render, settled } from '@ember/test-helpers';
@@ -463,35 +464,81 @@ module('Integration | Modifier Manager | class-based modifier', function (
 ) {
   setupRenderingTest(hooks);
 
-  testHooks(
-    (callback) =>
-      class NativeModifier extends Modifier {
-        constructor(owner: unknown, args: ModifierArgs) {
-          super(owner, args);
-          callback('constructor', this);
-        }
-
-        didReceiveArguments(): void {
-          callback('didReceiveArguments', this);
-        }
-
-        didUpdateArguments(): void {
-          callback('didUpdateArguments', this);
-        }
-
-        didInstall(): void {
-          callback('didInstall', this);
-        }
-
-        willRemove(): void {
-          callback('willRemove', this);
-        }
-
-        willDestroy(): void {
-          callback('willDestroy', this);
-        }
-      }
-  );
+  if (gte('3.22.0')) {
+    module('capabilities(3.22)', function () {
+      testHooks(
+        (callback) =>
+          class NativeModifier extends Modifier {
+            constructor(owner: unknown, args: ModifierArgs) {
+              super(owner, args);
+              callback('constructor', this);
+            }
+
+            didReceiveArguments(): void {
+              for (let i = 0; i < this.args.positional.length; i++) {
+                // "noop" / consume the arg
+                this.args.positional[i];
+              }
+
+              for (const key of Object.keys(this.args.named)) {
+                // "noop" / consume the arg
+                this.args.named[key];
+              }
+
+              callback('didReceiveArguments', this);
+            }
+
+            didUpdateArguments(): void {
+              callback('didUpdateArguments', this);
+            }
+
+            didInstall(): void {
+              callback('didInstall', this);
+            }
+
+            willRemove(): void {
+              callback('willRemove', this);
+            }
+
+            willDestroy(): void {
+              callback('willDestroy', this);
+            }
+          }
+      );
+    });
+  } else {
+    module('capabilities(3.13)', function () {
+      testHooks(
+        (callback) =>
+          class NativeModifier extends Modifier {
+            constructor(owner: unknown, args: ModifierArgs) {
+              super(owner, args);
+              callback('constructor', this);
+            }
+
+            didReceiveArguments(): void {
+              callback('didReceiveArguments', this);
+            }
+
+            didUpdateArguments(): void {
+              callback('didUpdateArguments', this);
+            }
+
+            didInstall(): void {
+              callback('didInstall', this);
+            }
+
+            willRemove(): void {
+              callback('willRemove', this);
+            }
+
+            willDestroy(): void {
+              callback('willDestroy', this);
+            }
+          }
+      );
+    });
+  }
 
   module('service injection', function () {
     test('can participate in ember dependency injection', async function (this: TestContext, assert) {

diff --git a/tests/integration/modifiers/functional-modifier-test.ts b/tests/integration/modifiers/functional-modifier-test.ts
@@ -1,3 +1,4 @@
+import { gte } from 'ember-compatibility-helpers';
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
 import { render, settled } from '@ember/test-helpers';
@@ -89,10 +90,17 @@ module('Integration | Modifiers | functional modifier', function (hooks) {
 
       this.registerModifier(
         'songbird',
-        modifier(() => callCount++)
+        modifier((_, positional: [number]) => {
+          // A change is not triggered unless args are consumed
+          if (gte('3.22.0')) {
+            positional[0];
+          }
+
+          return callCount++;
+        })
       );
 
-      await render(hbs`<h1 {{songbird value}}>Hello</h1>`);
+      await render(hbs`<h1 {{songbird this.value}}>Hello</h1>`);
 
       assert.equal(callCount, 1);
 
@@ -109,10 +117,17 @@ module('Integration | Modifiers | functional modifier', function (hooks) {
 
       this.registerModifier(
         'songbird',
-        modifier(() => () => callCount++)
+        modifier((_, positional: [number]) => {
+          // A change is not triggered unless args are consumed
+          if (gte('3.22.0')) {
+            positional[0];
+          }
+
+          return () => callCount++;
+        })
       );
 
-      await render(hbs`<h1 {{songbird value}}>Hello</h1>`);
+      await render(hbs`<h1 {{songbird this.value}}>Hello</h1>`);
 
       assert.equal(callCount, 0);
 

diff --git a/type-tests/modifier-test.ts b/type-tests/modifier-test.ts
@@ -3,13 +3,19 @@ import { expectTypeOf } from 'expect-type';
 import Modifier, { modifier, ModifierArgs } from 'ember-modifier';
 
 // --- function modifier --- //
-expectTypeOf(modifier).toEqualTypeOf<
+expectTypeOf(modifier).toMatchTypeOf<
   (
     callback: (
       element: Element,
       positional: unknown[],
       named: Record<string, unknown>
-    ) => (() => unknown) | void
+    ) =>
+      | ((
+          element: Element,
+          positional: unknown[],
+          named: Record<string, unknown>
+        ) => unknown)
+      | void
   ) => unknown
 >();