Fix issue with dynamic modifiers infinitely receiving duplicate arguments

Author: NullVoxPopuli

packages/@glimmer-workspace/integration-tests/test/modifiers/dynamic-modifiers-test.ts

@@ -65,6 +65,28 @@ class DynamicModifiersResolutionModeTest extends RenderTest {
     this.assertStableRerender();
   }
 
+  @test
+  'Modifiers with dynamic arguments receive the correct number of arguments'(assert) {
+    let receivedArgs: unknown[] = [];
+    const foo = defineSimpleModifier(
+      (_element: unknown, args: unknown[]) => (receivedArgs = args)
+    );
+
+    this.render(`
+      {{~#let (modifier this.foo this.outer) as |foo|~}}
+        <div {{ (if this.cond (modifier foo this.inner)) }}>General Kenobi!</div>
+      {{~/let~}}
+      `, { foo, inner: 'x', outer: 'y', cond: true });
+
+    this.assertHTML('<div>General Kenobi!</div>');
+    this.assertStableRerender();
+    assert.deepEqual(receivedArgs, ['y', 'x']);
+    this.rerender({ cond: false });
+    this.rerender({ cond: true });
+    assert.deepEqual(receivedArgs, ['y', 'x']);
+
+  }
+
   @test
   'Can pass curried modifier as argument and invoke dynamically (with args)'() {
     const foo = defineSimpleModifier(

packages/@glimmer-workspace/integration-tests/test/modifiers/on-test.ts

@@ -315,38 +315,6 @@ if (hasDom) {
       this.assertCounts({ adds: 1, removes: 0 });
     }
 
-    @test
-    'updates to a (remaining truthy) condition do not leave the element without an attached modifier'(assert:
-      Assert) {
-      let calledCount = 0;
-
-      this.render(`
-        <button {{
-          (if this.condition
-            (modifier this.on "click" this.callback)
-          )
-        }}>Click Me</button>`, {
-        callback() {
-          calledCount++;
-        },
-        on,
-        condition: true,
-      });
-
-      this.findButton().click();
-      assert.strictEqual(calledCount, 1, 'callback is being invoked');
-
-      this.rerender({ condition: true });
-
-      this.findButton().click();
-      assert.strictEqual(calledCount, 2, 'callback is being invoked');
-
-      this.rerender({ condition: true });
-
-      this.findButton().click();
-      assert.strictEqual(calledCount, 3, 'callback is being invoked');
-    }
-
     @test
     'asserts when eventName is missing'(assert: Assert) {
       assert.throws(() => {

packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts

@@ -22,7 +22,7 @@ import {
 import { associateDestroyableChild, destroy } from '@glimmer/destroyable';
 import { getInternalModifierManager } from '@glimmer/manager';
 import { createComputeRef, isConstRef, valueForRef } from '@glimmer/reference';
-import { assign, debugToString, expect, isObject } from '@glimmer/util';
+import { debugToString, expect, isObject } from '@glimmer/util';
 import { consumeTag, CURRENT_TAG, validateTag, valueForTag } from '@glimmer/validator';
 import { $t0, CurriedTypes, Op } from '@glimmer/vm';
 
@@ -153,6 +153,8 @@ APPEND_OPCODES.add(Op.DynamicModifier, (vm) => {
   let { stack } = vm;
   let ref = check(stack.pop(), CheckReference);
   let args = check(stack.pop(), CheckArguments).capture();
+  let { positional: outerPositional, named: outerNamed } = args;
+
   let { constructing } = vm.elements();
   let initialOwner = vm.getOwner();
 
@@ -178,11 +180,11 @@ APPEND_OPCODES.add(Op.DynamicModifier, (vm) => {
       owner = curriedOwner;
 
       if (positional !== undefined) {
-        args.positional = positional.concat(args.positional) as CapturedPositionalArguments;
+        args.positional = [...positional, ...outerPositional] as CapturedPositionalArguments;
       }
 
       if (named !== undefined) {
-        args.named = assign({}, ...named, args.named);
+        args.named = Object.assign({}, named, outerNamed);
       }
     } else {
       hostDefinition = value;
@@ -194,10 +196,8 @@ APPEND_OPCODES.add(Op.DynamicModifier, (vm) => {
     if (manager === null) {
       if (import.meta.env.DEV) {
         throw new Error(
-          `Expected a dynamic modifier definition, but received an object or function that did not have a modifier manager associated with it. The dynamic invocation was \`{{${
-            ref.debugLabel
-          }}}\`, and the incorrect definition is the value at the path \`${
-            ref.debugLabel
+          `Expected a dynamic modifier definition, but received an object or function that did not have a modifier manager associated with it. The dynamic invocation was \`{{${ref.debugLabel
+          }}}\`, and the incorrect definition is the value at the path \`${ref.debugLabel
           }\`, which was: ${debugToString!(hostDefinition)}`
         );
       } else {