fix(Common): allow null/undefined values for NgForTrackBy

Reverts a breaking change introduced in 2.4.1 by angular#13420 fixes angular#13641
quyentv88 · Jan 5, 2017 · f88cd2f · f88cd2f
1 parent f822f95
commit f88cd2f
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 10 deletions.
diff --git a/modules/@angular/common/src/directives/ng_for.ts b/modules/@angular/common/src/directives/ng_for.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ChangeDetectorRef, CollectionChangeRecord, DefaultIterableDiffer, Directive, DoCheck, EmbeddedViewRef, Input, IterableDiffer, IterableDiffers, OnChanges, SimpleChanges, TemplateRef, TrackByFn, ViewContainerRef} from '@angular/core';
+import {ChangeDetectorRef, CollectionChangeRecord, DefaultIterableDiffer, Directive, DoCheck, EmbeddedViewRef, Input, IterableDiffer, IterableDiffers, OnChanges, SimpleChanges, TemplateRef, TrackByFn, ViewContainerRef, isDevMode} from '@angular/core';
 
 import {getTypeNameForDebugging} from '../facade/lang';
 
@@ -91,9 +91,13 @@ export class NgFor implements DoCheck, OnChanges {
   @Input() ngForOf: any;
   @Input()
   set ngForTrackBy(fn: TrackByFn) {
-    if (typeof fn !== 'function') {
-      throw new Error(`trackBy must be a function, but received ${JSON.stringify(fn)}.
-      See https://angular.io/docs/ts/latest/api/common/index/NgFor-directive.html#!#change-propagation for more information.`);
+    if (isDevMode() && fn != null && typeof fn !== 'function') {
+      // TODO(vicb): use a log service once there is a public one available
+      if (<any>console && <any>console.warn) {
+        console.warn(
+            `trackBy must be a function, but received ${JSON.stringify(fn)}. ` +
+            `See https://angular.io/docs/ts/latest/api/common/index/NgFor-directive.html#!#change-propagation for more information.`);
+      }
     }
     this._trackByFn = fn;
   }

diff --git a/modules/@angular/common/test/directives/ng_for_spec.ts b/modules/@angular/common/test/directives/ng_for_spec.ts
@@ -294,14 +294,25 @@ export function main() {
        }));
 
     describe('track by', () => {
-      it('should throw if trackBy is not a function', async(() => {
+      it('should console.warn if trackBy is not a function', async(() => {
+           // TODO(vicb): expect a warning message when we have a proper log service
            const template =
-               `<template ngFor let-item [ngForOf]="items" [ngForTrackBy]="item?.id"></template>`;
+               `<template ngFor let-item [ngForOf]="items" [ngForTrackBy]="value"></template>`;
            fixture = createTestComponent(template);
+           fixture.componentInstance.value = 0;
+           fixture.detectChanges();
+         }));
 
-           getComponent().items = [{id: 1}, {id: 2}];
-           expect(() => fixture.detectChanges())
-               .toThrowError(/trackBy must be a function, but received null/);
+      it('should track by identity when trackBy is to `null` or `undefined`', async(() => {
+           // TODO(vicb): expect no warning message when we have a proper log service
+           const template =
+               `<template ngFor let-item [ngForOf]="items" [ngForTrackBy]="value">{{ item }}</template>`;
+           fixture = createTestComponent(template);
+           fixture.componentInstance.items = ['a', 'b', 'c'];
+           fixture.componentInstance.value = null;
+           detectChangesAndExpectText('abc');
+           fixture.componentInstance.value = undefined;
+           detectChangesAndExpectText('abc');
          }));
 
       it('should set the context to the component instance', async(() => {
@@ -343,6 +354,7 @@ export function main() {
            getComponent().items = [{'id': 'a', 'color': 'red'}];
            detectChangesAndExpectText('red');
          }));
+
       it('should move items around and keep them updated ', async(() => {
            const template =
                `<div><template ngFor let-item [ngForOf]="items" [ngForTrackBy]="trackById">{{item['color']}}</template></div>`;
@@ -378,6 +390,7 @@ class Foo {
 @Component({selector: 'test-cmp', template: ''})
 class TestComponent {
   @ContentChild(TemplateRef) contentTpl: TemplateRef<Object>;
+  value: any;
   items: any[] = [1, 2];
   trackById(index: number, item: any): string { return item['id']; }
   trackByIndex(index: number, item: any): number { return index; }
@@ -394,4 +407,4 @@ const TEMPLATE = '<div><span template="ngFor let item of items">{{item.toString(
 function createTestComponent(template: string = TEMPLATE): ComponentFixture<TestComponent> {
   return TestBed.overrideComponent(TestComponent, {set: {template: template}})
       .createComponent(TestComponent);
-}
+}