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

fix(core): markDirty() should only mark flags when really scheduling tick. (#39316) 

Close #39296

Fix an issue that `markDirty()` will not trigger change detection.

The case is for example we have the following component.

```
export class AppComponent implements OnInit {
  constructor(private router: Router) {}

  ngOnInit() {
    this.router.events
      .pipe(filter((e) => e instanceof NavigationEnd))
      .subscribe(() => ɵmarkDirty(this));
  }
}

export class CounterComponent implements OnInit, OnDestroy {
  ngOnInit() {
    this.countSubject.pipe(takeUntil(this.destroy)).subscribe((count) => {
      this.count = count;
      ɵmarkDirty(this);
    });
  }
```

Then the app navigate from `AppComponent` to `CounterComponent`,
so there are 2 `markDirty()` call at in a row.

The `1st` call is from `AppComponent` when router changed, the
`2nd` call is from `CounterComponent.ngOnInit()`.

And the `markDirty()->scheduleTick()` code look like this

```
function scheduleTick(rootContext, flags) {
    const nothingScheduled = rootContext.flags === 0 /* Empty */;
    rootContext.flags |= flags;
    if (nothingScheduled && rootContext.clean == _CLEAN_PROMISE) {
      rootContext.schedule(() => {
	...
        if (rootContext.flags & RootContextFlags.DetectChanges)
          rootContext.flags &= ~RootContextFlags.DetectChanges;
          tickContext();

        rootContext.clean = _CLEAN_PROMISE;
        ...
      });
```

So in this case, the `1st` markDirty() will
1. set rootContext.flags = 1
2. before `tickContext()`, reset rootContext.flags = 0
3. inside `tickContext()`, it will call `CounterComponent.ngOnint()`,
   so the `2nd` markDirty() is called.
4. and the `2nd` scheduleTick is called, `nothingScheduled` is true,
   but rootContext.clean is not `_CLEAN_PROMISE` yet, since the `1st` markDirty tick
   is still running.
5. So nowhere will reset the `rootContext.flags`.
6. then in the future, any other `markDirty()` call will not trigger the tick, since
   `nothingScheduled` is always false.

So `nothingScheduled` means no tick is scheduled, `rootContext.clean === _CLEAN_PROMISE`
means no tick is running.
So we should set the flags to `rootContext` only when `no tick is scheudled or running`.

PR Close #39316
This commit is contained in:
JiaLiPassion 2020-10-17 22:56:09 +09:00 committed by Joey Perrott
parent 170af0740d
commit 4a8d5ae970
2 changed files with 104 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1851,9 +1851,10 @@ export function markViewDirty(lView: LView): LView|null {
*/
export function scheduleTick(rootContext: RootContext, flags: RootContextFlags) {
const nothingScheduled = rootContext.flags === RootContextFlags.Empty;
rootContext.flags |= flags;
if (nothingScheduled && rootContext.clean == _CLEAN_PROMISE) {
// https://github.com/angular/angular/issues/39296
// should only attach the flags when really scheduling a tick
rootContext.flags |= flags;
let res: null|((val: null) => void);
rootContext.clean = new Promise<null>((r) => res = r);
rootContext.scheduler(() => {

104
packages/core/test/render3/change_detection_spec.ts
View File

@ -8,18 +8,20 @@
import {withBody} from '@angular/private/testing';
import {ChangeDetectionStrategy, DoCheck} from '../../src/core';
import {ChangeDetectionStrategy, DoCheck, OnInit} from '../../src/core';
import {whenRendered} from '../../src/render3/component';
import {getRenderedText, LifecycleHooksFeature, ɵɵadvance, ɵɵdefineComponent, ɵɵgetCurrentView, ɵɵproperty, ɵɵtextInterpolate1, ɵɵtextInterpolate2} from '../../src/render3/index';
import {detectChanges, markDirty, tick, ɵɵelement, ɵɵelementEnd, ɵɵelementStart, ɵɵlistener, ɵɵtext, ɵɵtextInterpolate} from '../../src/render3/instructions/all';
import {AttributeMarker, getRenderedText, LifecycleHooksFeature, ɵɵadvance, ɵɵdefineComponent, ɵɵgetCurrentView, ɵɵproperty, ɵɵtextInterpolate1, ɵɵtextInterpolate2} from '../../src/render3/index';
import {detectChanges, markDirty, tick, ɵɵelement, ɵɵelementEnd, ɵɵelementStart, ɵɵlistener, ɵɵtemplate, ɵɵtext, ɵɵtextInterpolate} from '../../src/render3/instructions/all';
import {RenderFlags} from '../../src/render3/interfaces/definition';
import {Renderer3, RendererFactory3} from '../../src/render3/interfaces/renderer';
import {FLAGS, LViewFlags} from '../../src/render3/interfaces/view';
import {NgIf} from './common_with_def';
import {containerEl, createComponent, renderComponent, requestAnimationFrame} from './render_util';
describe('change detection', () => {
describe('markDirty, detectChanges, whenRendered, getRenderedText', () => {
let mycompOninit: MyComponentWithOnInit;
class MyComponent implements DoCheck {
value: string = 'works';
doCheckCount = 0;
@ -48,6 +50,84 @@ describe('change detection', () => {
});
}
class MyComponentWithOnInit implements OnInit, DoCheck {
value: string = 'works';
doCheckCount = 0;
ngOnInit() {
markDirty(this);
}
ngDoCheck(): void {
this.doCheckCount++;
}
click() {
this.value = 'click works';
markDirty(this);
}
static ɵfac = () => mycompOninit = new MyComponentWithOnInit();
static ɵcmp = ɵɵdefineComponent({
type: MyComponentWithOnInit,
selectors: [['my-comp-oninit']],
decls: 2,
vars: 1,
template:
(rf: RenderFlags, ctx: MyComponentWithOnInit) => {
if (rf & RenderFlags.Create) {
ɵɵelementStart(0, 'span');
ɵɵtext(1);
ɵɵelementEnd();
}
if (rf & RenderFlags.Update) {
ɵɵadvance(1);
ɵɵtextInterpolate(ctx.value);
}
}
});
}
class MyParentComponent implements OnInit {
show = false;
value = 'parent';
mycomp: any = undefined;
ngOnInit() {}
click() {
this.show = true;
markDirty(this);
}
static ɵfac = () => new MyParentComponent();
static ɵcmp = ɵɵdefineComponent({
type: MyParentComponent,
selectors: [['my-parent-comp']],
decls: 2,
vars: 1,
directives: [NgIf, MyComponentWithOnInit],
consts: [[AttributeMarker.Template, 'ngIf']],
template:
(rf: RenderFlags, ctx: MyParentComponent) => {
if (rf & RenderFlags.Create) {
ɵɵtext(0, ' -->\n');
ɵɵtemplate(1, (rf, ctx) => {
if (rf & RenderFlags.Create) {
ɵɵelementStart(0, 'div');
ɵɵelement(1, 'my-comp-oninit');
ɵɵelementEnd();
}
}, 2, 0, 'div', 0);
}
if (rf & RenderFlags.Update) {
ɵɵadvance(1);
ɵɵproperty('ngIf', ctx.show);
}
}
});
}
it('should mark a component dirty and schedule change detection', withBody('my-comp', () => {
const myComp = renderComponent(MyComponent, {hostFeatures: [LifecycleHooksFeature]});
expect(getRenderedText(myComp)).toEqual('works');
@ -66,6 +146,24 @@ describe('change detection', () => {
expect(getRenderedText(myComp)).toEqual('updated');
}));
it('should detectChanges after markDirty is called multiple times within ngOnInit',
withBody('my-comp-oninit', () => {
const myParentComp =
renderComponent(MyParentComponent, {hostFeatures: [LifecycleHooksFeature]});
expect(myParentComp.show).toBe(false);
myParentComp.click();
requestAnimationFrame.flush();
expect(myParentComp.show).toBe(true);
const myComp = mycompOninit;
expect(getRenderedText(myComp)).toEqual('works');
expect(myComp.doCheckCount).toBe(1);
myComp.click();
expect(getRenderedText(myComp)).toEqual('works');
requestAnimationFrame.flush();
expect(getRenderedText(myComp)).toEqual('click works');
expect(myComp.doCheckCount).toBe(2);
}));
it('should detectChanges only once if markDirty is called multiple times',
withBody('my-comp', () => {
const myComp = renderComponent(MyComponent, {hostFeatures: [LifecycleHooksFeature]});