fix(compiler): allow nullable values in for loop block (#51997)

crisbeto · alxhub · commit 0eae992c4e03 · 2023-10-04T06:39:43.000-07:00
Updates the TCB for `@for` loop blocks to allow nullable values. The runtime already supports it and this makes it easier to switch from `NgFor`. Fixes #51993. PR Close #51997
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
@@ -1297,7 +1297,10 @@ class TcbForOfOp extends TcbOp {
     const loopScope = Scope.forNodes(this.tcb, this.scope, this.block, null);
     const initializer = ts.factory.createVariableDeclarationList(
         [ts.factory.createVariableDeclaration(this.block.item.name)], ts.NodeFlags.Const);
-    const expression = tcbExpression(this.block.expression, this.tcb, loopScope);
+    // It's common to have a for loop over a nullable value (e.g. produced by the `async` pipe).
+    // Add a non-null expression to allow such values to be assigned.
+    const expression = ts.factory.createNonNullExpression(
+        tcbExpression(this.block.expression, this.tcb, loopScope));
     const trackTranslator = new TcbForLoopTrackTranslator(this.tcb, loopScope, this.block);
     const trackExpression = trackTranslator.translate(this.block.trackBy);
     const statements = [
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
@@ -1485,7 +1485,7 @@ describe('type check blocks', () => {
       `;
 
       const result = tcb(TEMPLATE);
-      expect(result).toContain('for (const item of ((this).items)) { var _t1 = item;');
+      expect(result).toContain('for (const item of ((this).items)!) { var _t1 = item;');
       expect(result).toContain('"" + ((this).main(_t1))');
       expect(result).toContain('"" + ((this).empty())');
     });
@@ -1498,7 +1498,7 @@ describe('type check blocks', () => {
       `;
 
       const result = tcb(TEMPLATE);
-      expect(result).toContain('for (const item of ((this).items)) { var _t1 = item;');
+      expect(result).toContain('for (const item of ((this).items)!) { var _t1 = item;');
       expect(result).toContain('var _t2: number = null!;');
       expect(result).toContain('var _t3: number = null!;');
       expect(result).toContain('var _t4: number = null!;');
@@ -1516,7 +1516,7 @@ describe('type check blocks', () => {
       `;
 
       const result = tcb(TEMPLATE);
-      expect(result).toContain('for (const item of ((this).items)) { var _t1 = item;');
+      expect(result).toContain('for (const item of ((this).items)!) { var _t1 = item;');
       expect(result).toContain('var _t2: number = null!;');
       expect(result).toContain('var _t3: number = null!;');
       expect(result).toContain('var _t4: number = null!;');
@@ -1532,7 +1532,7 @@ describe('type check blocks', () => {
       `;
 
       const result = tcb(TEMPLATE);
-      expect(result).toContain('for (const item of ((this).items)) { var _t1 = item;');
+      expect(result).toContain('for (const item of ((this).items)!) { var _t1 = item;');
       expect(result).toContain('var _t2: number = null!;');
       expect(result).toContain('"" + (((this).$index)) + (_t2)');
     });
@@ -1550,18 +1550,18 @@ describe('type check blocks', () => {
 
       const result = tcb(TEMPLATE);
       expect(result).toContain(
-          'for (const item of ((this).items)) { var _t1 = item; var _t2: number = null!;');
+          'for (const item of ((this).items)!) { var _t1 = item; var _t2: number = null!;');
       expect(result).toContain('"" + (_t1) + (_t2)');
       expect(result).toContain(
-          'for (const inner of ((_t1).items)) { var _t8 = inner; var _t9: number = null!;');
+          'for (const inner of ((_t1).items)!) { var _t8 = inner; var _t9: number = null!;');
       expect(result).toContain('"" + (_t1) + (_t2) + (_t8) + (_t9)');
     });
 
     it('should generate the tracking expression of a for loop', () => {
       const result = tcb(`@for (item of items; track trackingFn($index, item, prop)) {}`);
 
       expect(result).toContain(
-          'for (const item of ((this).items)) { var _t1: number = null!; var _t2 = item;');
+          'for (const item of ((this).items)!) { var _t1: number = null!; var _t2 = item;');
       expect(result).toContain('(this).trackingFn(_t1, _t2, ((this).prop));');
     });
   });
diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
@@ -4495,6 +4495,57 @@ suppress
         expect(diags[0].messageText)
             .toContain('Error: Illegal State: Pipes are not allowed in this context');
       });
+
+      it('should allow nullable values in loop expression', () => {
+        env.write('test.ts', `
+          import {Component, Pipe} from '@angular/core';
+
+          @Pipe({name: 'fakeAsync', standalone: true})
+          export class FakeAsyncPipe {
+            transform<T>(value: Iterable<T>): Iterable<T> | null | undefined {
+              return null;
+            }
+          }
+
+          @Component({
+            template: \`
+              @for (item of items | fakeAsync; track item) {
+                {{item}}
+              }
+            \`,
+            standalone: true,
+            imports: [FakeAsyncPipe]
+          })
+          export class Main {
+            items = [];
+          }
+        `);
+
+        const diags = env.driveDiagnostics();
+        expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([]);
+      });
+
+      it('should enforce that the loop expression is iterable', () => {
+        env.write('test.ts', `
+          import {Component} from '@angular/core';
+
+          @Component({
+            template: \`
+              @for (item of items; track item) {
+                {{item}}
+              }
+            \`,
+          })
+          export class Main {
+            items = 123;
+          }
+        `);
+
+        const diags = env.driveDiagnostics();
+        expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
+          `Type 'number' must have a '[Symbol.iterator]()' method that returns an iterator.`
+        ]);
+      });
     });
   });
 });
