From 67868cf7bb044423708133cbdf68c2b14f5ffa49 Mon Sep 17 00:00:00 2001
From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com>
Date: Fri, 14 Mar 2025 20:09:01 -0400
Subject: [PATCH 1/5] oh

---
 .../transform/template-to-typescript.test.ts  |  9 ++
 .../template/template-to-typescript.ts        | 96 ++++++++++++++++---
 2 files changed, 93 insertions(+), 12 deletions(-)

diff --git a/packages/core/__tests__/transform/template-to-typescript.test.ts b/packages/core/__tests__/transform/template-to-typescript.test.ts
index 47b6a0287..e4b496207 100644
--- a/packages/core/__tests__/transform/template-to-typescript.test.ts
+++ b/packages/core/__tests__/transform/template-to-typescript.test.ts
@@ -836,6 +836,15 @@ describe('Transform: rewriteTemplate', () => {
         }"
       `);
     });
+    
+    test('in a conditional expression', () => {
+      let template = `{{(if @onSelect (modifier on "click" @onSelect))}}`;
+      let specialForms = { if: 'if' } as const;
+
+      expect(templateBody(template, { globals: ['on'], specialForms })).toMatchInlineSnapshot(`
+        "__glintDSL__.emitContent(__glintDSL__.resolveOrReturn((__glintDSL__.noop(__if), (__glintRef__.args.onSelect) ? (__glintDSL__.applyModifier(__glintDSL__.resolve(__glintDSL__.Globals["on"])(__glintY__.element, "click", __glintRef__.args.onSelect))) : (null)))());"
+      `);
+    });
   });
 
   describe('subexpressions', () => {
diff --git a/packages/core/src/transform/template/template-to-typescript.ts b/packages/core/src/transform/template/template-to-typescript.ts
index 64409e88c..cac720307 100644
--- a/packages/core/src/transform/template/template-to-typescript.ts
+++ b/packages/core/src/transform/template/template-to-typescript.ts
@@ -355,20 +355,92 @@ export function templateToTypescript(
           node.params.length >= 2,
           () => `{{${formInfo.name}}} requires at least two parameters`,
         );
-
-        mapper.text('(');
-        emitExpression(node.params[0]);
-        mapper.text(') ? (');
-        emitExpression(node.params[1]);
-        mapper.text(') : (');
-
-        if (node.params[2]) {
-          emitExpression(node.params[2]);
+        
+        // Check if this is a conditional modifier case
+        const isModifierHelper = 
+          node.params[1].type === 'SubExpression' && 
+          node.params[1].path.type === 'PathExpression' && 
+          node.params[1].path.original === 'modifier';
+        
+        if (isModifierHelper) {
+          // For conditional modifiers, we need to handle them specially
+          // to avoid type instantiation depth issues
+          mapper.text('(');
+          emitExpression(node.params[0]);
+          mapper.text(') ? (');
+          
+          // For the truthy case, we emit the modifier directly
+          if (node.params[1].type === 'SubExpression') {
+            // Extract the modifier and its arguments
+            const modifierExpr = node.params[1];
+            if (modifierExpr.params.length > 0 && modifierExpr.params[0].type === 'PathExpression') {
+              mapper.text('__glintDSL__.applyModifier(__glintDSL__.resolve(');
+              emitExpression(modifierExpr.params[0]);
+              mapper.text(')(__glintY__.element, ');
+              
+              // Skip the first param (the modifier itself) and emit the rest
+              const restParams = modifierExpr.params.slice(1);
+              for (let [index, param] of restParams.entries()) {
+                if (index > 0) {
+                  mapper.text(', ');
+                }
+                emitExpression(param);
+              }
+              
+              // Handle named args if any
+              if (modifierExpr.hash.pairs.length) {
+                if (restParams.length) {
+                  mapper.text(', ');
+                }
+                
+                mapper.text('{ ');
+                for (let [index, pair] of modifierExpr.hash.pairs.entries()) {
+                  mapper.text(`${pair.key}: `);
+                  emitExpression(pair.value);
+                  
+                  if (index < modifierExpr.hash.pairs.length - 1) {
+                    mapper.text(', ');
+                  }
+                }
+                mapper.text(' }');
+              }
+              
+              mapper.text('))');
+            } else {
+              // Fallback to normal emission if structure is unexpected
+              emitExpression(node.params[1]);
+            }
+          } else {
+            // Fallback to normal emission if structure is unexpected
+            emitExpression(node.params[1]);
+          }
+          
+          mapper.text(') : (');
+          
+          // For the falsy case, emit null or the else branch
+          if (node.params[2]) {
+            emitExpression(node.params[2]);
+          } else {
+            mapper.text('null');
+          }
+          
+          mapper.text(')');
         } else {
-          mapper.text('undefined');
+          // Standard if expression handling (unchanged)
+          mapper.text('(');
+          emitExpression(node.params[0]);
+          mapper.text(') ? (');
+          emitExpression(node.params[1]);
+          mapper.text(') : (');
+          
+          if (node.params[2]) {
+            emitExpression(node.params[2]);
+          } else {
+            mapper.text('undefined');
+          }
+          
+          mapper.text(')');
         }
-
-        mapper.text(')');
       });
     }
 

From e500c56efa5a190552400ea572504a14c1f7f8f0 Mon Sep 17 00:00:00 2001
From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com>
Date: Fri, 14 Mar 2025 20:41:11 -0400
Subject: [PATCH 2/5] Test if test fails

---
 .../template/template-to-typescript.ts        | 96 +++----------------
 1 file changed, 12 insertions(+), 84 deletions(-)

diff --git a/packages/core/src/transform/template/template-to-typescript.ts b/packages/core/src/transform/template/template-to-typescript.ts
index cac720307..64409e88c 100644
--- a/packages/core/src/transform/template/template-to-typescript.ts
+++ b/packages/core/src/transform/template/template-to-typescript.ts
@@ -355,92 +355,20 @@ export function templateToTypescript(
           node.params.length >= 2,
           () => `{{${formInfo.name}}} requires at least two parameters`,
         );
-        
-        // Check if this is a conditional modifier case
-        const isModifierHelper = 
-          node.params[1].type === 'SubExpression' && 
-          node.params[1].path.type === 'PathExpression' && 
-          node.params[1].path.original === 'modifier';
-        
-        if (isModifierHelper) {
-          // For conditional modifiers, we need to handle them specially
-          // to avoid type instantiation depth issues
-          mapper.text('(');
-          emitExpression(node.params[0]);
-          mapper.text(') ? (');
-          
-          // For the truthy case, we emit the modifier directly
-          if (node.params[1].type === 'SubExpression') {
-            // Extract the modifier and its arguments
-            const modifierExpr = node.params[1];
-            if (modifierExpr.params.length > 0 && modifierExpr.params[0].type === 'PathExpression') {
-              mapper.text('__glintDSL__.applyModifier(__glintDSL__.resolve(');
-              emitExpression(modifierExpr.params[0]);
-              mapper.text(')(__glintY__.element, ');
-              
-              // Skip the first param (the modifier itself) and emit the rest
-              const restParams = modifierExpr.params.slice(1);
-              for (let [index, param] of restParams.entries()) {
-                if (index > 0) {
-                  mapper.text(', ');
-                }
-                emitExpression(param);
-              }
-              
-              // Handle named args if any
-              if (modifierExpr.hash.pairs.length) {
-                if (restParams.length) {
-                  mapper.text(', ');
-                }
-                
-                mapper.text('{ ');
-                for (let [index, pair] of modifierExpr.hash.pairs.entries()) {
-                  mapper.text(`${pair.key}: `);
-                  emitExpression(pair.value);
-                  
-                  if (index < modifierExpr.hash.pairs.length - 1) {
-                    mapper.text(', ');
-                  }
-                }
-                mapper.text(' }');
-              }
-              
-              mapper.text('))');
-            } else {
-              // Fallback to normal emission if structure is unexpected
-              emitExpression(node.params[1]);
-            }
-          } else {
-            // Fallback to normal emission if structure is unexpected
-            emitExpression(node.params[1]);
-          }
-          
-          mapper.text(') : (');
-          
-          // For the falsy case, emit null or the else branch
-          if (node.params[2]) {
-            emitExpression(node.params[2]);
-          } else {
-            mapper.text('null');
-          }
-          
-          mapper.text(')');
+
+        mapper.text('(');
+        emitExpression(node.params[0]);
+        mapper.text(') ? (');
+        emitExpression(node.params[1]);
+        mapper.text(') : (');
+
+        if (node.params[2]) {
+          emitExpression(node.params[2]);
         } else {
-          // Standard if expression handling (unchanged)
-          mapper.text('(');
-          emitExpression(node.params[0]);
-          mapper.text(') ? (');
-          emitExpression(node.params[1]);
-          mapper.text(') : (');
-          
-          if (node.params[2]) {
-            emitExpression(node.params[2]);
-          } else {
-            mapper.text('undefined');
-          }
-          
-          mapper.text(')');
+          mapper.text('undefined');
         }
+
+        mapper.text(')');
       });
     }
 

From 9ca7ee4534149b210c5e296dd82ca38959699051 Mon Sep 17 00:00:00 2001
From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com>
Date: Fri, 14 Mar 2025 20:46:10 -0400
Subject: [PATCH 3/5] Revert "Test if test fails"

This reverts commit e500c56efa5a190552400ea572504a14c1f7f8f0.
---
 .../template/template-to-typescript.ts        | 96 ++++++++++++++++---
 1 file changed, 84 insertions(+), 12 deletions(-)

diff --git a/packages/core/src/transform/template/template-to-typescript.ts b/packages/core/src/transform/template/template-to-typescript.ts
index 64409e88c..cac720307 100644
--- a/packages/core/src/transform/template/template-to-typescript.ts
+++ b/packages/core/src/transform/template/template-to-typescript.ts
@@ -355,20 +355,92 @@ export function templateToTypescript(
           node.params.length >= 2,
           () => `{{${formInfo.name}}} requires at least two parameters`,
         );
-
-        mapper.text('(');
-        emitExpression(node.params[0]);
-        mapper.text(') ? (');
-        emitExpression(node.params[1]);
-        mapper.text(') : (');
-
-        if (node.params[2]) {
-          emitExpression(node.params[2]);
+        
+        // Check if this is a conditional modifier case
+        const isModifierHelper = 
+          node.params[1].type === 'SubExpression' && 
+          node.params[1].path.type === 'PathExpression' && 
+          node.params[1].path.original === 'modifier';
+        
+        if (isModifierHelper) {
+          // For conditional modifiers, we need to handle them specially
+          // to avoid type instantiation depth issues
+          mapper.text('(');
+          emitExpression(node.params[0]);
+          mapper.text(') ? (');
+          
+          // For the truthy case, we emit the modifier directly
+          if (node.params[1].type === 'SubExpression') {
+            // Extract the modifier and its arguments
+            const modifierExpr = node.params[1];
+            if (modifierExpr.params.length > 0 && modifierExpr.params[0].type === 'PathExpression') {
+              mapper.text('__glintDSL__.applyModifier(__glintDSL__.resolve(');
+              emitExpression(modifierExpr.params[0]);
+              mapper.text(')(__glintY__.element, ');
+              
+              // Skip the first param (the modifier itself) and emit the rest
+              const restParams = modifierExpr.params.slice(1);
+              for (let [index, param] of restParams.entries()) {
+                if (index > 0) {
+                  mapper.text(', ');
+                }
+                emitExpression(param);
+              }
+              
+              // Handle named args if any
+              if (modifierExpr.hash.pairs.length) {
+                if (restParams.length) {
+                  mapper.text(', ');
+                }
+                
+                mapper.text('{ ');
+                for (let [index, pair] of modifierExpr.hash.pairs.entries()) {
+                  mapper.text(`${pair.key}: `);
+                  emitExpression(pair.value);
+                  
+                  if (index < modifierExpr.hash.pairs.length - 1) {
+                    mapper.text(', ');
+                  }
+                }
+                mapper.text(' }');
+              }
+              
+              mapper.text('))');
+            } else {
+              // Fallback to normal emission if structure is unexpected
+              emitExpression(node.params[1]);
+            }
+          } else {
+            // Fallback to normal emission if structure is unexpected
+            emitExpression(node.params[1]);
+          }
+          
+          mapper.text(') : (');
+          
+          // For the falsy case, emit null or the else branch
+          if (node.params[2]) {
+            emitExpression(node.params[2]);
+          } else {
+            mapper.text('null');
+          }
+          
+          mapper.text(')');
         } else {
-          mapper.text('undefined');
+          // Standard if expression handling (unchanged)
+          mapper.text('(');
+          emitExpression(node.params[0]);
+          mapper.text(') ? (');
+          emitExpression(node.params[1]);
+          mapper.text(') : (');
+          
+          if (node.params[2]) {
+            emitExpression(node.params[2]);
+          } else {
+            mapper.text('undefined');
+          }
+          
+          mapper.text(')');
         }
-
-        mapper.text(')');
       });
     }
 

From c96c29ac037c3ebcb9d9d7c2b771c9c9293a4f1e Mon Sep 17 00:00:00 2001
From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com>
Date: Fri, 14 Mar 2025 21:23:30 -0400
Subject: [PATCH 4/5] Add test case for #812

---
 .../test-cases/conditionall-modifier.gts      | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 test-packages/ts-ember-app/app/components/test-cases/conditionall-modifier.gts

diff --git a/test-packages/ts-ember-app/app/components/test-cases/conditionall-modifier.gts b/test-packages/ts-ember-app/app/components/test-cases/conditionall-modifier.gts
new file mode 100644
index 000000000..299fcb9b4
--- /dev/null
+++ b/test-packages/ts-ember-app/app/components/test-cases/conditionall-modifier.gts
@@ -0,0 +1,20 @@
+import { on } from "@ember/modifier";
+import type { TOC } from "@ember/component/template-only";
+
+export interface ItemSignature {
+  Element: HTMLButtonElement;
+  Args: { onSelect?: (event: Event) => void };
+  Blocks: { default: [] };
+}
+
+// https://github.com/typed-ember/glint/issues/812
+export const Item: TOC<ItemSignature> = <template>
+  <button
+    type="button"
+    role="menuitem"
+    {{(if @onSelect (modifier on "click" @onSelect))}}
+    ...attributes
+  >
+    {{yield}}
+  </button>
+</template>;

From f59dbba40342e56de4ad2757199d9bf685e53b0c Mon Sep 17 00:00:00 2001
From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com>
Date: Fri, 14 Mar 2025 22:05:59 -0400
Subject: [PATCH 5/5] Prettier

---
 .../transform/template-to-typescript.test.ts  |  2 +-
 .../template/template-to-typescript.ts        | 37 ++++++++++---------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/packages/core/__tests__/transform/template-to-typescript.test.ts b/packages/core/__tests__/transform/template-to-typescript.test.ts
index e4b496207..4e6b8671c 100644
--- a/packages/core/__tests__/transform/template-to-typescript.test.ts
+++ b/packages/core/__tests__/transform/template-to-typescript.test.ts
@@ -836,7 +836,7 @@ describe('Transform: rewriteTemplate', () => {
         }"
       `);
     });
-    
+
     test('in a conditional expression', () => {
       let template = `{{(if @onSelect (modifier on "click" @onSelect))}}`;
       let specialForms = { if: 'if' } as const;
diff --git a/packages/core/src/transform/template/template-to-typescript.ts b/packages/core/src/transform/template/template-to-typescript.ts
index cac720307..6364b8916 100644
--- a/packages/core/src/transform/template/template-to-typescript.ts
+++ b/packages/core/src/transform/template/template-to-typescript.ts
@@ -355,29 +355,32 @@ export function templateToTypescript(
           node.params.length >= 2,
           () => `{{${formInfo.name}}} requires at least two parameters`,
         );
-        
+
         // Check if this is a conditional modifier case
-        const isModifierHelper = 
-          node.params[1].type === 'SubExpression' && 
-          node.params[1].path.type === 'PathExpression' && 
+        const isModifierHelper =
+          node.params[1].type === 'SubExpression' &&
+          node.params[1].path.type === 'PathExpression' &&
           node.params[1].path.original === 'modifier';
-        
+
         if (isModifierHelper) {
           // For conditional modifiers, we need to handle them specially
           // to avoid type instantiation depth issues
           mapper.text('(');
           emitExpression(node.params[0]);
           mapper.text(') ? (');
-          
+
           // For the truthy case, we emit the modifier directly
           if (node.params[1].type === 'SubExpression') {
             // Extract the modifier and its arguments
             const modifierExpr = node.params[1];
-            if (modifierExpr.params.length > 0 && modifierExpr.params[0].type === 'PathExpression') {
+            if (
+              modifierExpr.params.length > 0 &&
+              modifierExpr.params[0].type === 'PathExpression'
+            ) {
               mapper.text('__glintDSL__.applyModifier(__glintDSL__.resolve(');
               emitExpression(modifierExpr.params[0]);
               mapper.text(')(__glintY__.element, ');
-              
+
               // Skip the first param (the modifier itself) and emit the rest
               const restParams = modifierExpr.params.slice(1);
               for (let [index, param] of restParams.entries()) {
@@ -386,25 +389,25 @@ export function templateToTypescript(
                 }
                 emitExpression(param);
               }
-              
+
               // Handle named args if any
               if (modifierExpr.hash.pairs.length) {
                 if (restParams.length) {
                   mapper.text(', ');
                 }
-                
+
                 mapper.text('{ ');
                 for (let [index, pair] of modifierExpr.hash.pairs.entries()) {
                   mapper.text(`${pair.key}: `);
                   emitExpression(pair.value);
-                  
+
                   if (index < modifierExpr.hash.pairs.length - 1) {
                     mapper.text(', ');
                   }
                 }
                 mapper.text(' }');
               }
-              
+
               mapper.text('))');
             } else {
               // Fallback to normal emission if structure is unexpected
@@ -414,16 +417,16 @@ export function templateToTypescript(
             // Fallback to normal emission if structure is unexpected
             emitExpression(node.params[1]);
           }
-          
+
           mapper.text(') : (');
-          
+
           // For the falsy case, emit null or the else branch
           if (node.params[2]) {
             emitExpression(node.params[2]);
           } else {
             mapper.text('null');
           }
-          
+
           mapper.text(')');
         } else {
           // Standard if expression handling (unchanged)
@@ -432,13 +435,13 @@ export function templateToTypescript(
           mapper.text(') ? (');
           emitExpression(node.params[1]);
           mapper.text(') : (');
-          
+
           if (node.params[2]) {
             emitExpression(node.params[2]);
           } else {
             mapper.text('undefined');
           }
-          
+
           mapper.text(')');
         }
       });