From 243e40d21f062bde4136bae1e7abd79bcfe4d28d Mon Sep 17 00:00:00 2001
From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com>
Date: Mon, 17 Mar 2025 17:38:39 -0400
Subject: [PATCH 1/2] Devin: try to fix inline if helper bug

---
 .../test/helpers/if-inline-test.ts            | 212 ++++++++++++++++++
 .../lib/compiled/opcodes/expressions.ts       |   5 +-
 2 files changed, 216 insertions(+), 1 deletion(-)
 create mode 100644 packages/@glimmer-workspace/integration-tests/test/helpers/if-inline-test.ts

diff --git a/packages/@glimmer-workspace/integration-tests/test/helpers/if-inline-test.ts b/packages/@glimmer-workspace/integration-tests/test/helpers/if-inline-test.ts
new file mode 100644
index 000000000..54be12926
--- /dev/null
+++ b/packages/@glimmer-workspace/integration-tests/test/helpers/if-inline-test.ts
@@ -0,0 +1,212 @@
+import {
+  GlimmerishComponent,
+  jitSuite,
+  RenderTest,
+  strip,
+  test,
+  tracked,
+} from '@glimmer-workspace/integration-tests';
+
+class IfInlineTest extends RenderTest {
+  static suiteName = 'Helpers test: inline {{if}}';
+
+  @test
+  'inline if helper updates when tracked property changes'() {
+    class Person {
+      @tracked isActive = false;
+      
+      toggle() {
+        this.isActive = !this.isActive;
+      }
+    }
+
+    const person = new Person();
+
+    this.render(
+      strip`<div>{{if this.person.isActive "Active" "Inactive"}}</div>`,
+      { person }
+    );
+
+    this.assertHTML('<div>Inactive</div>', 'Initial render shows inactive state');
+    this.assertStableRerender();
+
+    person.toggle();
+    this.rerender();
+    this.assertHTML('<div>Active</div>', 'Updates when tracked property changes to true');
+
+    person.toggle();
+    this.rerender();
+    this.assertHTML('<div>Inactive</div>', 'Updates when tracked property changes to false');
+  }
+
+  @test
+  'inline if helper with only truthy value updates when tracked property changes'() {
+    class Person {
+      @tracked isActive = false;
+      
+      toggle() {
+        this.isActive = !this.isActive;
+      }
+    }
+
+    const person = new Person();
+
+    this.render(
+      strip`<div>{{if this.person.isActive "Active"}}</div>`,
+      { person }
+    );
+
+    this.assertHTML('<div></div>', 'Initial render shows empty when false');
+    this.assertStableRerender();
+
+    person.toggle();
+    this.rerender();
+    this.assertHTML('<div>Active</div>', 'Updates when tracked property changes to true');
+
+    person.toggle();
+    this.rerender();
+    this.assertHTML('<div></div>', 'Updates when tracked property changes to false');
+  }
+
+  @test
+  'inline if helper updates when component argument changes'() {
+    let testComponent: TestComponent;
+    
+    class TestComponent extends GlimmerishComponent {
+      @tracked isActive = false;
+      
+      constructor(owner: object, args: Record<string, unknown>) {
+        super(owner, args);
+        testComponent = this;
+      }
+    }
+
+    this.registerComponent(
+      'Glimmer',
+      'TestComponent',
+      '{{if @isActive "Active" "Inactive"}}',
+      TestComponent
+    );
+
+    this.render(
+      strip`<TestComponent @isActive={{this.isActive}} />`,
+      { isActive: false }
+    );
+
+    this.assertHTML('Inactive', 'Initial render shows inactive state');
+    this.assertStableRerender();
+
+    this.rerender({ isActive: true });
+    this.assertHTML('Active', 'Updates when argument changes to true');
+
+    this.rerender({ isActive: false });
+    this.assertHTML('Inactive', 'Updates when argument changes to false');
+  }
+
+  @test
+  'inline if helper with components updates when tracked property changes'() {
+    class Person {
+      @tracked isActive = false;
+      
+      toggle() {
+        this.isActive = !this.isActive;
+      }
+    }
+
+    const person = new Person();
+
+    class OkComponent extends GlimmerishComponent {}
+    class KoComponent extends GlimmerishComponent {}
+
+    this.registerComponent('Glimmer', 'Ok', '<div>OK Component</div>', OkComponent);
+    this.registerComponent('Glimmer', 'Ko', '<div>KO Component</div>', KoComponent);
+
+    this.render(
+      strip`<div>{{if this.person.isActive Ok Ko}}</div>`,
+      { person }
+    );
+
+    this.assertHTML('<div><div>KO Component</div></div>', 'Initial render shows KO component');
+    this.assertStableRerender();
+
+    person.toggle();
+    this.rerender();
+    this.assertHTML('<div><div>OK Component</div></div>', 'Updates to OK component when tracked property changes to true');
+
+    person.toggle();
+    this.rerender();
+    this.assertHTML('<div><div>KO Component</div></div>', 'Updates to KO component when tracked property changes to false');
+  }
+
+  @test
+  'inline if helper with components updates when component argument changes'() {
+    class OkComponent extends GlimmerishComponent {}
+    class KoComponent extends GlimmerishComponent {}
+
+    this.registerComponent('Glimmer', 'Ok', '<div>OK Component</div>', OkComponent);
+    this.registerComponent('Glimmer', 'Ko', '<div>KO Component</div>', KoComponent);
+
+    this.registerComponent(
+      'Glimmer',
+      'TestComponent',
+      '{{if @isOk Ok Ko}}',
+      class extends GlimmerishComponent {}
+    );
+
+    this.render(
+      strip`<TestComponent @isOk={{this.isOk}} />`,
+      { isOk: false }
+    );
+
+    this.assertHTML('<div>KO Component</div>', 'Initial render shows KO component');
+    this.assertStableRerender();
+
+    this.rerender({ isOk: true });
+    this.assertHTML('<div>OK Component</div>', 'Updates to OK component when argument changes to true');
+
+    this.rerender({ isOk: false });
+    this.assertHTML('<div>KO Component</div>', 'Updates to KO component when argument changes to false');
+  }
+
+  @test
+  'comparison with block form if helper using components'() {
+    class Person {
+      @tracked isActive = false;
+      
+      toggle() {
+        this.isActive = !this.isActive;
+      }
+    }
+
+    const person = new Person();
+
+    class OkComponent extends GlimmerishComponent {}
+    class KoComponent extends GlimmerishComponent {}
+
+    this.registerComponent('Glimmer', 'Ok', '<div>OK Component</div>', OkComponent);
+    this.registerComponent('Glimmer', 'Ko', '<div>KO Component</div>', KoComponent);
+
+    this.render(
+      strip`
+        <div>
+          Inline: {{if this.person.isActive Ok Ko}}
+          Block: {{#if this.person.isActive}}<Ok />{{else}}<Ko />{{/if}}
+        </div>
+      `,
+      { person }
+    );
+
+    this.assertHTML('<div>Inline: <div>KO Component</div> Block: <div>KO Component</div></div>', 'Initial render both show KO component');
+    this.assertStableRerender();
+
+    person.toggle();
+    this.rerender();
+    this.assertHTML('<div>Inline: <div>OK Component</div> Block: <div>OK Component</div></div>', 'Both update to OK component when tracked property changes to true');
+
+    person.toggle();
+    this.rerender();
+    this.assertHTML('<div>Inline: <div>KO Component</div> Block: <div>KO Component</div></div>', 'Both update to KO component when tracked property changes to false');
+  }
+}
+
+jitSuite(IfInlineTest);
diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts
index c27b0ca51..8703971c6 100644
--- a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts
+++ b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts
@@ -285,7 +285,10 @@ APPEND_OPCODES.add(VM_IF_INLINE_OP, (vm) => {
 
   vm.stack.push(
     createComputeRef(() => {
-      if (toBool(valueForRef(condition))) {
+      // Explicitly consume the condition reference to ensure tracking
+      let conditionValue = valueForRef(condition);
+      
+      if (toBool(conditionValue)) {
         return valueForRef(truthy);
       } else {
         return valueForRef(falsy);

From 2cda1f4ebfbc00b56fae1e517439efe66bf80ecc Mon Sep 17 00:00:00 2001
From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com>
Date: Mon, 17 Mar 2025 21:29:22 -0400
Subject: [PATCH 2/2] Changes after being told to run tests

---
 .../test/helpers/if-inline-test.ts            | 143 ++++++++----------
 .../lib/compiled/opcodes/expressions.ts       |  11 +-
 2 files changed, 72 insertions(+), 82 deletions(-)

diff --git a/packages/@glimmer-workspace/integration-tests/test/helpers/if-inline-test.ts b/packages/@glimmer-workspace/integration-tests/test/helpers/if-inline-test.ts
index 54be12926..0d48fd49c 100644
--- a/packages/@glimmer-workspace/integration-tests/test/helpers/if-inline-test.ts
+++ b/packages/@glimmer-workspace/integration-tests/test/helpers/if-inline-test.ts
@@ -1,5 +1,5 @@
 import {
-  GlimmerishComponent,
+  defineComponent,
   jitSuite,
   RenderTest,
   strip,
@@ -14,7 +14,7 @@ class IfInlineTest extends RenderTest {
   'inline if helper updates when tracked property changes'() {
     class Person {
       @tracked isActive = false;
-      
+
       toggle() {
         this.isActive = !this.isActive;
       }
@@ -22,10 +22,7 @@ class IfInlineTest extends RenderTest {
 
     const person = new Person();
 
-    this.render(
-      strip`<div>{{if this.person.isActive "Active" "Inactive"}}</div>`,
-      { person }
-    );
+    this.render(strip`<div>{{if this.person.isActive "Active" "Inactive"}}</div>`, { person });
 
     this.assertHTML('<div>Inactive</div>', 'Initial render shows inactive state');
     this.assertStableRerender();
@@ -43,7 +40,7 @@ class IfInlineTest extends RenderTest {
   'inline if helper with only truthy value updates when tracked property changes'() {
     class Person {
       @tracked isActive = false;
-      
+
       toggle() {
         this.isActive = !this.isActive;
       }
@@ -51,10 +48,7 @@ class IfInlineTest extends RenderTest {
 
     const person = new Person();
 
-    this.render(
-      strip`<div>{{if this.person.isActive "Active"}}</div>`,
-      { person }
-    );
+    this.render(strip`<div>{{if this.person.isActive "Active"}}</div>`, { person });
 
     this.assertHTML('<div></div>', 'Initial render shows empty when false');
     this.assertStableRerender();
@@ -70,36 +64,20 @@ class IfInlineTest extends RenderTest {
 
   @test
   'inline if helper updates when component argument changes'() {
-    let testComponent: TestComponent;
-    
-    class TestComponent extends GlimmerishComponent {
-      @tracked isActive = false;
-      
-      constructor(owner: object, args: Record<string, unknown>) {
-        super(owner, args);
-        testComponent = this;
-      }
-    }
+    const TestComponent = defineComponent({}, '{{if @isActive "Active" "Inactive"}}');
 
-    this.registerComponent(
-      'Glimmer',
-      'TestComponent',
-      '{{if @isActive "Active" "Inactive"}}',
-      TestComponent
-    );
-
-    this.render(
-      strip`<TestComponent @isActive={{this.isActive}} />`,
-      { isActive: false }
-    );
+    this.render('<TestComponent @isActive={{this.isActive}} />', {
+      isActive: false,
+      TestComponent,
+    });
 
     this.assertHTML('Inactive', 'Initial render shows inactive state');
     this.assertStableRerender();
 
-    this.rerender({ isActive: true });
+    this.rerender({ isActive: true, TestComponent });
     this.assertHTML('Active', 'Updates when argument changes to true');
 
-    this.rerender({ isActive: false });
+    this.rerender({ isActive: false, TestComponent });
     this.assertHTML('Inactive', 'Updates when argument changes to false');
   }
 
@@ -107,7 +85,7 @@ class IfInlineTest extends RenderTest {
   'inline if helper with components updates when tracked property changes'() {
     class Person {
       @tracked isActive = false;
-      
+
       toggle() {
         this.isActive = !this.isActive;
       }
@@ -115,64 +93,62 @@ class IfInlineTest extends RenderTest {
 
     const person = new Person();
 
-    class OkComponent extends GlimmerishComponent {}
-    class KoComponent extends GlimmerishComponent {}
+    const Ok = defineComponent({}, '<div>OK Component</div>');
+    const Ko = defineComponent({}, '<div>KO Component</div>');
 
-    this.registerComponent('Glimmer', 'Ok', '<div>OK Component</div>', OkComponent);
-    this.registerComponent('Glimmer', 'Ko', '<div>KO Component</div>', KoComponent);
+    // Create a component with Ok and Ko in scope
+    const TestContainer = defineComponent({ Ok, Ko }, '<div>{{if @isActive Ok Ko}}</div>');
 
-    this.render(
-      strip`<div>{{if this.person.isActive Ok Ko}}</div>`,
-      { person }
-    );
+    this.render('<TestContainer @isActive={{this.person.isActive}} />', { person, TestContainer });
 
     this.assertHTML('<div><div>KO Component</div></div>', 'Initial render shows KO component');
     this.assertStableRerender();
 
     person.toggle();
     this.rerender();
-    this.assertHTML('<div><div>OK Component</div></div>', 'Updates to OK component when tracked property changes to true');
+    this.assertHTML(
+      '<div><div>OK Component</div></div>',
+      'Updates to OK component when tracked property changes to true'
+    );
 
     person.toggle();
     this.rerender();
-    this.assertHTML('<div><div>KO Component</div></div>', 'Updates to KO component when tracked property changes to false');
+    this.assertHTML(
+      '<div><div>KO Component</div></div>',
+      'Updates to KO component when tracked property changes to false'
+    );
   }
 
   @test
   'inline if helper with components updates when component argument changes'() {
-    class OkComponent extends GlimmerishComponent {}
-    class KoComponent extends GlimmerishComponent {}
-
-    this.registerComponent('Glimmer', 'Ok', '<div>OK Component</div>', OkComponent);
-    this.registerComponent('Glimmer', 'Ko', '<div>KO Component</div>', KoComponent);
+    const Ok = defineComponent({}, '<div>OK Component</div>');
+    const Ko = defineComponent({}, '<div>KO Component</div>');
 
-    this.registerComponent(
-      'Glimmer',
-      'TestComponent',
-      '{{if @isOk Ok Ko}}',
-      class extends GlimmerishComponent {}
-    );
+    const TestComponent = defineComponent({ Ok, Ko }, '{{if @isOk Ok Ko}}');
 
-    this.render(
-      strip`<TestComponent @isOk={{this.isOk}} />`,
-      { isOk: false }
-    );
+    this.render('<TestComponent @isOk={{this.isOk}} />', { isOk: false, TestComponent });
 
     this.assertHTML('<div>KO Component</div>', 'Initial render shows KO component');
     this.assertStableRerender();
 
-    this.rerender({ isOk: true });
-    this.assertHTML('<div>OK Component</div>', 'Updates to OK component when argument changes to true');
+    this.rerender({ isOk: true, TestComponent });
+    this.assertHTML(
+      '<div>OK Component</div>',
+      'Updates to OK component when argument changes to true'
+    );
 
-    this.rerender({ isOk: false });
-    this.assertHTML('<div>KO Component</div>', 'Updates to KO component when argument changes to false');
+    this.rerender({ isOk: false, TestComponent });
+    this.assertHTML(
+      '<div>KO Component</div>',
+      'Updates to KO component when argument changes to false'
+    );
   }
 
   @test
   'comparison with block form if helper using components'() {
     class Person {
       @tracked isActive = false;
-      
+
       toggle() {
         this.isActive = !this.isActive;
       }
@@ -180,32 +156,41 @@ class IfInlineTest extends RenderTest {
 
     const person = new Person();
 
-    class OkComponent extends GlimmerishComponent {}
-    class KoComponent extends GlimmerishComponent {}
-
-    this.registerComponent('Glimmer', 'Ok', '<div>OK Component</div>', OkComponent);
-    this.registerComponent('Glimmer', 'Ko', '<div>KO Component</div>', KoComponent);
+    const Ok = defineComponent({}, '<div>OK Component</div>');
+    const Ko = defineComponent({}, '<div>KO Component</div>');
 
-    this.render(
-      strip`
+    // Create a component with Ok and Ko in scope for both inline and block forms
+    const TestContainer = defineComponent(
+      { Ok, Ko },
+      `
         <div>
-          Inline: {{if this.person.isActive Ok Ko}}
-          Block: {{#if this.person.isActive}}<Ok />{{else}}<Ko />{{/if}}
+          Inline: {{if @isActive Ok Ko}}
+          Block: {{#if @isActive}}<Ok />{{else}}<Ko />{{/if}}
         </div>
-      `,
-      { person }
+      `
     );
 
-    this.assertHTML('<div>Inline: <div>KO Component</div> Block: <div>KO Component</div></div>', 'Initial render both show KO component');
+    this.render('<TestContainer @isActive={{this.person.isActive}} />', { person, TestContainer });
+
+    this.assertHTML(
+      '<div>Inline: <div>KO Component</div> Block: <div>KO Component</div></div>',
+      'Initial render both show KO component'
+    );
     this.assertStableRerender();
 
     person.toggle();
     this.rerender();
-    this.assertHTML('<div>Inline: <div>OK Component</div> Block: <div>OK Component</div></div>', 'Both update to OK component when tracked property changes to true');
+    this.assertHTML(
+      '<div>Inline: <div>OK Component</div> Block: <div>OK Component</div></div>',
+      'Both update to OK component when tracked property changes to true'
+    );
 
     person.toggle();
     this.rerender();
-    this.assertHTML('<div>Inline: <div>KO Component</div> Block: <div>KO Component</div></div>', 'Both update to KO component when tracked property changes to false');
+    this.assertHTML(
+      '<div>Inline: <div>KO Component</div> Block: <div>KO Component</div></div>',
+      'Both update to KO component when tracked property changes to false'
+    );
   }
 }
 
diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts
index 8703971c6..0e6b7aa48 100644
--- a/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts
+++ b/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts
@@ -287,11 +287,16 @@ APPEND_OPCODES.add(VM_IF_INLINE_OP, (vm) => {
     createComputeRef(() => {
       // Explicitly consume the condition reference to ensure tracking
       let conditionValue = valueForRef(condition);
-      
+
+      // Also explicitly consume truthy and falsy references to ensure proper tracking
+      // when they are component references
+      let truthyValue = valueForRef(truthy);
+      let falsyValue = valueForRef(falsy);
+
       if (toBool(conditionValue)) {
-        return valueForRef(truthy);
+        return truthyValue;
       } else {
-        return valueForRef(falsy);
+        return falsyValue;
       }
     })
   );