From f88cd2f22e011216a6e70a1bbbb69cdc62217957 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 3 Jan 2017 15:14:30 -0800 Subject: [PATCH] fix(Common): allow null/undefined values for `NgForTrackBy` Reverts a breaking change introduced in 2.4.1 by #13420 fixes #13641 --- .../@angular/common/src/directives/ng_for.ts | 12 ++++++--- .../common/test/directives/ng_for_spec.ts | 25 ++++++++++++++----- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/modules/@angular/common/src/directives/ng_for.ts b/modules/@angular/common/src/directives/ng_for.ts index f830b43120..0f837403eb 100644 --- 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 (console && 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 index e9bbdd55d2..284bc27320 100644 --- 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 = - ``; + ``; 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 = + ``; + 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 = `
`; @@ -378,6 +390,7 @@ class Foo { @Component({selector: 'test-cmp', template: ''}) class TestComponent { @ContentChild(TemplateRef) contentTpl: TemplateRef; + 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 = '
{{item.toString( function createTestComponent(template: string = TEMPLATE): ComponentFixture { return TestBed.overrideComponent(TestComponent, {set: {template: template}}) .createComponent(TestComponent); -} +} \ No newline at end of file