honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): do not reset view dirty state in check no changes mode (#34495) 

Unlike in View Engine, we currently reset the dirty state of
components in the check no changes change detection cycle.

This means that components cannot be marked as dirty from
view lifecycle hooks because the dirty state is reset and
the lifecycle hooks do not run in the check no changes CD cycle.

PR Close #34495
This commit is contained in:
Paul Gschwendtner 2020-01-11 16:23:11 +01:00 committed by atscott
parent ebcd59ae4f
commit 8de3c20e50
3 changed files with 114 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
packages/core/src/render3/instructions/shared.ts
View File

@ -384,6 +384,7 @@ export function refreshView<T>(
const flags = lView[FLAGS];
if ((flags & LViewFlags.Destroyed) === LViewFlags.Destroyed) return;
enterView(lView, lView[T_HOST]);
const checkNoChangesMode = getCheckNoChangesMode();
try {
resetPreOrderHookFlags(lView);
@ -392,11 +393,10 @@ export function refreshView<T>(
executeTemplate(lView, templateFn, RenderFlags.Update, context);
}
const checkNoChangesMode = getCheckNoChangesMode();
const hooksInitPhaseCompleted =
(flags & LViewFlags.InitPhaseStateMask) === InitPhaseState.InitPhaseCompleted;
// execute pre-order hooks (OnInit, OnChanges, DoChanges)
// execute pre-order hooks (OnInit, OnChanges, DoCheck)
// PERF WARNING: do NOT extract this to a separate function without running benchmarks
if (!checkNoChangesMode) {
if (hooksInitPhaseCompleted) {
@ -475,7 +475,17 @@ export function refreshView<T>(
if (tView.firstUpdatePass === true) {
tView.firstUpdatePass = false;
}
lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
// Do not reset the dirty state when running in check no changes mode. We don't want components
// to behave differently depending on whether check no changes is enabled or not. For example:
// Marking an OnPush component as dirty from within the `ngAfterViewInit` hook in order to
// refresh a `NgClass` binding should work. If we would reset the dirty state in the check
// no changes cycle, the component would be not be dirty for the next update pass. This would
// be different in production mode where the component dirty state is not reset.
if (!checkNoChangesMode) {
lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
}
leaveViewProcessExit();
}
}

103
packages/core/test/acceptance/change_detection_spec.ts
View File

@ -9,10 +9,10 @@
import {CommonModule} from '@angular/common';
import {ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, Directive, DoCheck, EmbeddedViewRef, ErrorHandler, Input, NgModule, OnInit, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core';
import {AfterContentChecked, AfterViewChecked} from '@angular/core/src/core';
import {AfterViewChecked} from '@angular/core/src/core';
import {ComponentFixture, TestBed} from '@angular/core/testing';
import {expect} from '@angular/platform-browser/testing/src/matchers';
import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
import {ivyEnabled} from '@angular/private/testing';
import {BehaviorSubject} from 'rxjs';
describe('change detection', () => {
@ -1249,6 +1249,105 @@ describe('change detection', () => {
});
});
describe('OnPush markForCheck in lifecycle hooks', () => {
describe('with check no changes enabled', () => createOnPushMarkForCheckTests(true));
describe('with check no changes disabled', () => createOnPushMarkForCheckTests(false));
function createOnPushMarkForCheckTests(checkNoChanges: boolean) {
const detectChanges = (f: ComponentFixture<any>) => f.detectChanges(checkNoChanges);
// 1. ngAfterViewInit and ngAfterViewChecked lifecycle hooks run after "OnPushComp" has
// been refreshed. They can mark the component as dirty. Meaning that the "OnPushComp"
// can be checked/refreshed in a subsequent change detection cycle.
// 2. ngDoCheck and ngAfterContentChecked lifecycle hooks run before "OnPushComp" is
// refreshed. This means that those hooks cannot leave the component as dirty because
// the dirty state is reset afterwards. Though these hooks run every change detection
// cycle before "OnPushComp" is considered for refreshing. Hence marking as dirty from
// within such a hook can cause the component to checked/refreshed as intended.
['ngAfterViewInit', 'ngAfterViewChecked', 'ngAfterContentChecked', 'ngDoCheck'].forEach(
hookName => {
it(`should be able to mark component as dirty from within ${hookName}`, () => {
@Component({
selector: 'on-push-comp',
changeDetection: ChangeDetectionStrategy.OnPush,
template: `<p>{{text}}</p>`,
})
class OnPushComp {
text = 'initial';
constructor(private _cdRef: ChangeDetectorRef){}
[hookName]() {
this._cdRef.markForCheck();
}
}
@Component({template: `<on-push-comp></on-push-comp>`})
class TestApp {
@ViewChild(OnPushComp) onPushComp !: OnPushComp;
}
TestBed.configureTestingModule(
{declarations: [TestApp, OnPushComp], imports: [CommonModule]});
const fixture = TestBed.createComponent(TestApp);
const pElement = fixture.nativeElement.querySelector('p') as HTMLElement;
detectChanges(fixture);
expect(pElement.textContent).toBe('initial');
// "OnPushComp" component should be re-checked since it has been left dirty
// in the first change detection (through the lifecycle hook). Hence, setting
// a programmatic value and triggering a new change detection cycle should cause
// the text to be updated in the view.
fixture.componentInstance.onPushComp.text = 'new';
detectChanges(fixture);
expect(pElement.textContent).toBe('new');
});
});
// ngOnInit and ngAfterContentInit lifecycle hooks run once before "OnPushComp" is
// refreshed/checked. This means they cannot mark the component as dirty because the
// component dirty state will immediately reset after these hooks complete.
['ngOnInit', 'ngAfterContentInit'].forEach(hookName => {
it(`should not be able to mark component as dirty from within ${hookName}`, () => {
@Component({
selector: 'on-push-comp',
changeDetection: ChangeDetectionStrategy.OnPush,
template: `<p>{{text}}</p>`,
})
class OnPushComp {
text = 'initial';
constructor(private _cdRef: ChangeDetectorRef){}
[hookName]() {
this._cdRef.markForCheck();
}
}
@Component({template: `<on-push-comp></on-push-comp>`})
class TestApp {
@ViewChild(OnPushComp) onPushComp !: OnPushComp;
}
TestBed.configureTestingModule(
{declarations: [TestApp, OnPushComp], imports: [CommonModule]});
const fixture = TestBed.createComponent(TestApp);
const pElement = fixture.nativeElement.querySelector('p') as HTMLElement;
detectChanges(fixture);
expect(pElement.textContent).toBe('initial');
fixture.componentInstance.onPushComp.text = 'new';
// this is a noop since the "OnPushComp" component is not marked as dirty. The
// programmatically updated value will not be reflected in the rendered view.
detectChanges(fixture);
expect(pElement.textContent).toBe('initial');
});
});
}
});
describe('ExpressionChangedAfterItHasBeenCheckedError', () => {
@Component({template: '...'})
class MyApp {

1
packages/core/test/acceptance/integration_spec.ts
View File

@ -9,7 +9,6 @@ import {CommonModule} from '@angular/common';
import {Component, ContentChild, Directive, ElementRef, EventEmitter, HostBinding, HostListener, Input, NgModule, OnInit, Output, Pipe, QueryList, TemplateRef, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core';
import {TVIEW} from '@angular/core/src/render3/interfaces/view';
import {getLView} from '@angular/core/src/render3/state';
import {loadLContext} from '@angular/core/src/render3/util/discovery_utils';
import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode';
import {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';