From 08a18b82de003c379aa76323e65e8230e5b42a56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mis=CC=8Cko=20Hevery?= Date: Fri, 13 Apr 2018 17:11:21 -0700 Subject: [PATCH] refactor(common): Remove ngOnChanges from NgForOf (#23378) `NgForOf` used to implement `OnChanges` and than use `ngOnChanges` callback to detect when `ngForOf` binding changed to update the differ. We now do the checking manually which puts less pressure on the runtime to do the bookkeeping and should result in minor perf improvement. PR Close #23378 --- packages/common/src/directives/ng_for_of.ts | 26 +++++++++++-------- .../bundling/todo/bundle.golden_symbols.json | 9 ------- packages/core/test/bundling/todo/index.ts | 5 ---- packages/core/test/render3/common_with_def.ts | 1 - tools/public_api_guard/common/common.d.ts | 3 +-- 5 files changed, 16 insertions(+), 28 deletions(-) diff --git a/packages/common/src/directives/ng_for_of.ts b/packages/common/src/directives/ng_for_of.ts index 7bffebc700..93288dc9f4 100644 --- a/packages/common/src/directives/ng_for_of.ts +++ b/packages/common/src/directives/ng_for_of.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ChangeDetectorRef, Directive, DoCheck, EmbeddedViewRef, Input, IterableChangeRecord, IterableChanges, IterableDiffer, IterableDiffers, NgIterable, OnChanges, SimpleChanges, TemplateRef, TrackByFunction, ViewContainerRef, forwardRef, isDevMode} from '@angular/core'; +import {ChangeDetectorRef, Directive, DoCheck, EmbeddedViewRef, Input, IterableChangeRecord, IterableChanges, IterableDiffer, IterableDiffers, NgIterable, TemplateRef, TrackByFunction, ViewContainerRef, forwardRef, isDevMode} from '@angular/core'; export class NgForOfContext { constructor( @@ -93,8 +93,12 @@ export class NgForOfContext { * */ @Directive({selector: '[ngFor][ngForOf]'}) -export class NgForOf implements DoCheck, OnChanges { - @Input() ngForOf: NgIterable; +export class NgForOf implements DoCheck { + @Input() + set ngForOf(ngForOf: NgIterable) { + this._ngForOf = ngForOf; + this._ngForOfDirty = true; + } @Input() set ngForTrackBy(fn: TrackByFunction) { if (isDevMode() && fn != null && typeof fn !== 'function') { @@ -110,6 +114,8 @@ export class NgForOf implements DoCheck, OnChanges { get ngForTrackBy(): TrackByFunction { return this._trackByFn; } + private _ngForOf: NgIterable; + private _ngForOfDirty: boolean = true; private _differ: IterableDiffer|null = null; private _trackByFn: TrackByFunction; @@ -127,10 +133,11 @@ export class NgForOf implements DoCheck, OnChanges { } } - ngOnChanges(changes: SimpleChanges): void { - if ('ngForOf' in changes) { + ngDoCheck(): void { + if (this._ngForOfDirty) { + this._ngForOfDirty = false; // React on ngForOf changes only once all inputs have been initialized - const value = changes['ngForOf'].currentValue; + const value = this._ngForOf; if (!this._differ && value) { try { this._differ = this._differs.find(value).create(this.ngForTrackBy); @@ -140,11 +147,8 @@ export class NgForOf implements DoCheck, OnChanges { } } } - } - - ngDoCheck(): void { if (this._differ) { - const changes = this._differ.diff(this.ngForOf); + const changes = this._differ.diff(this._ngForOf); if (changes) this._applyChanges(changes); } } @@ -155,7 +159,7 @@ export class NgForOf implements DoCheck, OnChanges { (item: IterableChangeRecord, adjustedPreviousIndex: number, currentIndex: number) => { if (item.previousIndex == null) { const view = this._viewContainer.createEmbeddedView( - this._template, new NgForOfContext(null !, this.ngForOf, -1, -1), currentIndex); + this._template, new NgForOfContext(null !, this._ngForOf, -1, -1), currentIndex); const tuple = new RecordViewTuple(item, view); insertTuples.push(tuple); } else if (currentIndex == null) { diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index fccb3b48e3..6571af19de 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -59,27 +59,18 @@ { "name": "NgIfContext" }, - { - "name": "NgOnChangesFeature" - }, { "name": "Optional" }, { "name": "PARAMETERS" }, - { - "name": "PRIVATE_PREFIX" - }, { "name": "ROOT_DIRECTIVE_INDICES" }, { "name": "RecordViewTuple" }, - { - "name": "SimpleChange" - }, { "name": "SkipSelf" }, diff --git a/packages/core/test/bundling/todo/index.ts b/packages/core/test/bundling/todo/index.ts index d5694a3407..579335ce45 100644 --- a/packages/core/test/bundling/todo/index.ts +++ b/packages/core/test/bundling/todo/index.ts @@ -160,11 +160,6 @@ if (!(NgIf as any).ngDirectiveDef) { selectors: [['', 'ngFor', '', 'ngForOf', '']], factory: () => new NgForOf( injectViewContainerRef(), injectTemplateRef(), directiveInject(IterableDiffers)), - features: [NgOnChangesFeature({ - ngForOf: 'ngForOf', - ngForTrackBy: 'ngForTrackBy', - ngForTemplate: 'ngForTemplate', - })], inputs: { ngForOf: 'ngForOf', ngForTrackBy: 'ngForTrackBy', diff --git a/packages/core/test/render3/common_with_def.ts b/packages/core/test/render3/common_with_def.ts index 41975dad0e..9f3742fd3b 100644 --- a/packages/core/test/render3/common_with_def.ts +++ b/packages/core/test/render3/common_with_def.ts @@ -20,7 +20,6 @@ NgForOf.ngDirectiveDef = defineDirective({ selectors: [['', 'ngForOf', '']], factory: () => new NgForOfDef( injectViewContainerRef(), injectTemplateRef(), directiveInject(IterableDiffers)), - features: [NgOnChangesFeature()], inputs: { ngForOf: 'ngForOf', ngForTrackBy: 'ngForTrackBy', diff --git a/tools/public_api_guard/common/common.d.ts b/tools/public_api_guard/common/common.d.ts index 0ba2b7ba56..1c04dc1595 100644 --- a/tools/public_api_guard/common/common.d.ts +++ b/tools/public_api_guard/common/common.d.ts @@ -240,13 +240,12 @@ export declare class NgComponentOutlet implements OnChanges, OnDestroy { ngOnDestroy(): void; } -export declare class NgForOf implements DoCheck, OnChanges { +export declare class NgForOf implements DoCheck { ngForOf: NgIterable; ngForTemplate: TemplateRef>; ngForTrackBy: TrackByFunction; constructor(_viewContainer: ViewContainerRef, _template: TemplateRef>, _differs: IterableDiffers); ngDoCheck(): void; - ngOnChanges(changes: SimpleChanges): void; } export declare class NgForOfContext {