Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1863,9 +1863,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;
JiaLiPassion marked this conversation as resolved.
Show resolved Hide resolved
let res: null|((val: null) => void);
rootContext.clean = new Promise<null>((r) => res = r);
rootContext.scheduler(() => {
Expand Down
104 changes: 101 additions & 3 deletions packages/core/test/render3/change_detection_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
Expand All @@ -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]});
Expand Down