Skip to content

Commit 896cf35

Browse files
matskoalxhub
authored andcommitted
fix(ivy): ensure renderer begin/end methods are only called during change detection (angular#28192)
In VE the renderer.begin() and renderer.end() methods are only called when CD is called on an element. This patch ensures that Ivy does the same thing. Jira issue: FW-945 PR Close angular#28192
1 parent 1f7d3b9 commit 896cf35

File tree

4 files changed

+45
-49
lines changed

4 files changed

+45
-49
lines changed

packages/core/src/render3/component_ref.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,6 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
169169
let component: T;
170170
let tElementNode: TElementNode;
171171
try {
172-
if (rendererFactory.begin) rendererFactory.begin();
173-
174172
const componentView = createRootComponentView(
175173
hostRNode, this.componentDef, rootLView, rendererFactory, renderer);
176174

@@ -216,7 +214,6 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
216214
refreshDescendantViews(rootLView);
217215
} finally {
218216
leaveView(oldLView);
219-
if (rendererFactory.end) rendererFactory.end();
220217
}
221218

222219
const componentRef = new ComponentRef(

packages/core/src/render3/instructions.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -395,12 +395,13 @@ function renderComponentOrTemplate<T>(
395395
const rendererFactory = hostView[RENDERER_FACTORY];
396396
const oldView = enterView(hostView, hostView[HOST_NODE]);
397397
const normalExecutionPath = !getCheckNoChangesMode();
398+
const creationModeIsActive = isCreationMode(hostView);
398399
try {
399-
if (normalExecutionPath && rendererFactory.begin) {
400+
if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) {
400401
rendererFactory.begin();
401402
}
402403

403-
if (isCreationMode(hostView)) {
404+
if (creationModeIsActive) {
404405
// creation mode pass
405406
if (templateFn) {
406407
namespaceHTML();
@@ -415,7 +416,7 @@ function renderComponentOrTemplate<T>(
415416
templateFn && templateFn(RenderFlags.Update, context !);
416417
refreshDescendantViews(hostView);
417418
} finally {
418-
if (normalExecutionPath && rendererFactory.end) {
419+
if (normalExecutionPath && !creationModeIsActive && rendererFactory.end) {
419420
rendererFactory.end();
420421
}
421422
leaveView(oldView);

packages/core/test/animation/animation_query_integration_spec.ts

Lines changed: 38 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -803,58 +803,56 @@ import {HostListener} from '../../src/metadata/directives';
803803
expect(player.element.style.height).toEqual('444px');
804804
});
805805

806-
fixmeIvy(
807-
'FW-945 - Ivy createComponent calls CD while VE waits for CD to be explicitly called')
808-
.it('should find newly inserted items in the component via :enter', () => {
809-
@Component({
810-
selector: 'ani-cmp',
811-
template: `
806+
it('should find newly inserted items in the component via :enter', () => {
807+
@Component({
808+
selector: 'ani-cmp',
809+
template: `
812810
<div @myAnimation>
813811
<div *ngFor="let item of items" class="child">
814812
{{ item }}
815813
</div>
816814
</div>
817815
`,
818-
animations: [trigger(
819-
'myAnimation',
820-
[
821-
transition(
822-
':enter',
823-
[
824-
query(
825-
':enter',
826-
[
827-
style({opacity: 0}),
828-
animate(1000, style({opacity: .5})),
829-
]),
830-
]),
831-
])]
832-
})
833-
class Cmp {
834-
public items: any[] = [0, 1, 2];
835-
}
816+
animations: [trigger(
817+
'myAnimation',
818+
[
819+
transition(
820+
':enter',
821+
[
822+
query(
823+
':enter',
824+
[
825+
style({opacity: 0}),
826+
animate(1000, style({opacity: .5})),
827+
]),
828+
]),
829+
])]
830+
})
831+
class Cmp {
832+
public items: any[] = [0, 1, 2];
833+
}
836834

837-
TestBed.configureTestingModule({declarations: [Cmp]});
835+
TestBed.configureTestingModule({declarations: [Cmp]});
838836

839-
const engine = TestBed.get(ɵAnimationEngine);
840-
const fixture = TestBed.createComponent(Cmp);
841-
const cmp = fixture.componentInstance;
837+
const engine = TestBed.get(ɵAnimationEngine);
838+
const fixture = TestBed.createComponent(Cmp);
839+
const cmp = fixture.componentInstance;
842840

843-
fixture.detectChanges();
844-
engine.flush();
841+
fixture.detectChanges();
842+
engine.flush();
845843

846-
const players = getLog();
847-
expect(players.length).toEqual(3);
844+
const players = getLog();
845+
expect(players.length).toEqual(3);
848846

849-
const [p1, p2, p3] = players;
850-
expect(p1.element.innerText.trim()).toEqual('0');
851-
expect(p2.element.innerText.trim()).toEqual('1');
852-
expect(p3.element.innerText.trim()).toEqual('2');
847+
const [p1, p2, p3] = players;
848+
expect(p1.element.innerText.trim()).toEqual('0');
849+
expect(p2.element.innerText.trim()).toEqual('1');
850+
expect(p3.element.innerText.trim()).toEqual('2');
853851

854-
players.forEach(p => {
855-
expect(p.keyframes).toEqual([{opacity: '0', offset: 0}, {opacity: '0.5', offset: 1}]);
856-
});
857-
});
852+
players.forEach(p => {
853+
expect(p.keyframes).toEqual([{opacity: '0', offset: 0}, {opacity: '0.5', offset: 1}]);
854+
});
855+
});
858856

859857
it('should cleanup :enter and :leave artifacts from nodes when any animation sequences fail to be built',
860858
() => {

packages/core/test/render3/renderer_factory_spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ describe('renderer factory lifecycle', () => {
105105

106106
it('should work with a template', () => {
107107
renderToHtml(Template, {}, 1, 0, null, null, rendererFactory);
108-
expect(logs).toEqual(['create', 'begin', 'function create', 'function update', 'end']);
108+
expect(logs).toEqual(['create', 'function create', 'function update']);
109109

110110
logs = [];
111111
renderToHtml(Template, {});
@@ -115,8 +115,8 @@ describe('renderer factory lifecycle', () => {
115115
it('should work with a template which contains a component', () => {
116116
renderToHtml(TemplateWithComponent, {}, 2, 0, directives, null, rendererFactory);
117117
expect(logs).toEqual([
118-
'create', 'begin', 'function_with_component create', 'create', 'component create',
119-
'function_with_component update', 'component update', 'end'
118+
'create', 'function_with_component create', 'create', 'component create',
119+
'function_with_component update', 'component update'
120120
]);
121121

122122
logs = [];

0 commit comments

Comments
 (0)