Merge pull request #1565 from glimmerjs/dont-duplicate-positional-arg…

…uments-when-modifiers-update

Don't infinitely duplicate positional arguments when modifiers update

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: 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/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.concat(outerPositional) as CapturedPositionalArguments;
       }
 
       if (named !== undefined) {
-        args.named = assign({}, ...named, args.named);
+        args.named = Object.assign({}, ...named, outerNamed);
       }
     } else {
       hostDefinition = value;