Skip to content

Commit a20b2f7

Browse files
karamatsko
authored andcommitted
fix(ivy): process creation mode deeply before running update mode (angular#27744)
Prior to this commit, we had two different modes for change detection execution for Ivy, depending on whether you called `bootstrap()` or `renderComponent()`. In the former case, we would complete creation mode for all components in the tree before beginning update mode for any component. In the latter case, we would run creation mode and update mode together for each component individually. Maintaining code to support these two different execution orders was unnecessarily complex, so this commit aligns the two bootstrapping mechanisms to execute in the same order. Now creation mode always runs for all components before update mode begins. This change also simplifies our rendering logic so that we use `LView` flags as the source of truth for rendering mode instead of `rf` function arguments. This fixed some related bugs (e.g. calling `ViewRef.detectChanges` synchronously after the view's creation would create view nodes twice, view queries would execute twice, etc). PR Close angular#27744
1 parent f48a00f commit a20b2f7

25 files changed

+384
-303
lines changed

packages/common/test/BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ ts_library(
1515
"//packages/platform-browser",
1616
"//packages/platform-browser-dynamic",
1717
"//packages/platform-browser/testing",
18+
"//packages/private/testing",
1819
],
1920
)
2021

packages/common/test/directives/ng_component_outlet_spec.ts

+14-10
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {NgComponentOutlet} from '@angular/common/src/directives/ng_component_out
1111
import {Compiler, Component, ComponentRef, Inject, InjectionToken, Injector, NO_ERRORS_SCHEMA, NgModule, NgModuleFactory, Optional, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core';
1212
import {TestBed, async} from '@angular/core/testing';
1313
import {expect} from '@angular/platform-browser/testing/src/matchers';
14+
import {modifiedInIvy} from '@angular/private/testing';
1415

1516
describe('insert/remove', () => {
1617

@@ -106,17 +107,20 @@ describe('insert/remove', () => {
106107

107108
}));
108109

109-
it('should resolve a with injector', async(() => {
110-
let fixture = TestBed.createComponent(TestComponent);
111110

112-
fixture.componentInstance.cmpRef = null;
113-
fixture.componentInstance.currentComponent = InjectedComponent;
114-
fixture.detectChanges();
115-
let cmpRef: ComponentRef<InjectedComponent> = fixture.componentInstance.cmpRef !;
116-
expect(cmpRef).toBeAnInstanceOf(ComponentRef);
117-
expect(cmpRef.instance).toBeAnInstanceOf(InjectedComponent);
118-
expect(cmpRef.instance.testToken).toBeNull();
119-
}));
111+
modifiedInIvy('Static ViewChild and ContentChild queries are resolved in update mode')
112+
.it('should resolve with an injector', async(() => {
113+
let fixture = TestBed.createComponent(TestComponent);
114+
115+
// We are accessing a ViewChild (ngComponentOutlet) before change detection has run
116+
fixture.componentInstance.cmpRef = null;
117+
fixture.componentInstance.currentComponent = InjectedComponent;
118+
fixture.detectChanges();
119+
let cmpRef: ComponentRef<InjectedComponent> = fixture.componentInstance.cmpRef !;
120+
expect(cmpRef).toBeAnInstanceOf(ComponentRef);
121+
expect(cmpRef.instance).toBeAnInstanceOf(InjectedComponent);
122+
expect(cmpRef.instance.testToken).toBeNull();
123+
}));
120124

121125
it('should render projectable nodes, if supplied', async(() => {
122126
const template = `<ng-template>projected foo</ng-template>${TEST_CMP_TEMPLATE}`;

packages/core/src/render3/bindings.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {devModeEqual} from '../change_detection/change_detection_util';
1111
import {assertDataInRange, assertLessThan, assertNotEqual} from './assert';
1212
import {throwErrorIfNoChangesMode} from './errors';
1313
import {BINDING_INDEX, LView} from './interfaces/view';
14-
import {getCheckNoChangesMode, getCreationMode} from './state';
14+
import {getCheckNoChangesMode, isCreationMode} from './state';
1515
import {NO_CHANGE} from './tokens';
1616
import {isDifferent} from './util';
1717

@@ -44,7 +44,7 @@ export function bindingUpdated(lView: LView, bindingIndex: number, value: any):
4444
} else if (isDifferent(lView[bindingIndex], value)) {
4545
if (ngDevMode && getCheckNoChangesMode()) {
4646
if (!devModeEqual(lView[bindingIndex], value)) {
47-
throwErrorIfNoChangesMode(getCreationMode(), lView[bindingIndex], value);
47+
throwErrorIfNoChangesMode(isCreationMode(lView), lView[bindingIndex], value);
4848
}
4949
}
5050
lView[bindingIndex] = value;

packages/core/src/render3/component.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {ComponentDef, ComponentType, RenderFlags} from './interfaces/definition'
2222
import {TElementNode, TNode, TNodeFlags, TNodeType} from './interfaces/node';
2323
import {PlayerHandler} from './interfaces/player';
2424
import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from './interfaces/renderer';
25-
import {CONTEXT, HEADER_OFFSET, HOST, HOST_NODE, LView, LViewFlags, RootContext, RootContextFlags, TVIEW} from './interfaces/view';
25+
import {CONTEXT, FLAGS, HEADER_OFFSET, HOST, HOST_NODE, LView, LViewFlags, RootContext, RootContextFlags, TVIEW} from './interfaces/view';
2626
import {enterView, getPreviousOrParentTNode, leaveView, resetComponentState, setCurrentDirectiveDef} from './state';
2727
import {defaultScheduler, getRootView, readPatchedLView, stringify} from './util';
2828

@@ -133,7 +133,9 @@ export function renderComponent<T>(
133133
component = createRootComponent(
134134
componentView, componentDef, rootView, rootContext, opts.hostFeatures || null);
135135

136-
refreshDescendantViews(rootView, null);
136+
refreshDescendantViews(rootView); // creation mode pass
137+
rootView[FLAGS] &= ~LViewFlags.CreationMode;
138+
refreshDescendantViews(rootView); // update mode pass
137139
} finally {
138140
leaveView(oldView);
139141
if (rendererFactory.end) rendererFactory.end();

packages/core/src/render3/component_ref.ts

+2-3
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,9 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
208208
componentView, this.componentDef, rootLView, rootContext, [LifecycleHooksFeature]);
209209

210210
addToViewTree(rootLView, HEADER_OFFSET, componentView);
211-
212-
refreshDescendantViews(rootLView, RenderFlags.Create);
211+
refreshDescendantViews(rootLView);
213212
} finally {
214-
leaveView(oldLView, true);
213+
leaveView(oldLView);
215214
if (rendererFactory.end) rendererFactory.end();
216215
}
217216

packages/core/src/render3/hooks.ts

+11-8
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,10 @@ function queueDestroyHooks(def: DirectiveDef<any>, tView: TView, i: number): voi
9393
*
9494
* @param currentView The current view
9595
*/
96-
export function executeInitHooks(currentView: LView, tView: TView, creationMode: boolean): void {
97-
if (currentView[FLAGS] & LViewFlags.RunInit) {
98-
executeHooks(currentView, tView.initHooks, tView.checkHooks, creationMode);
96+
export function executeInitHooks(
97+
currentView: LView, tView: TView, checkNoChangesMode: boolean): void {
98+
if (!checkNoChangesMode && currentView[FLAGS] & LViewFlags.RunInit) {
99+
executeHooks(currentView, tView.initHooks, tView.checkHooks, checkNoChangesMode);
99100
currentView[FLAGS] &= ~LViewFlags.RunInit;
100101
}
101102
}
@@ -106,17 +107,19 @@ export function executeInitHooks(currentView: LView, tView: TView, creationMode:
106107
* @param currentView The current view
107108
*/
108109
export function executeHooks(
109-
data: LView, allHooks: HookData | null, checkHooks: HookData | null,
110-
creationMode: boolean): void {
111-
const hooksToCall = creationMode ? allHooks : checkHooks;
110+
currentView: LView, allHooks: HookData | null, checkHooks: HookData | null,
111+
checkNoChangesMode: boolean): void {
112+
if (checkNoChangesMode) return;
113+
114+
const hooksToCall = currentView[FLAGS] & LViewFlags.FirstLViewPass ? allHooks : checkHooks;
112115
if (hooksToCall) {
113-
callHooks(data, hooksToCall);
116+
callHooks(currentView, hooksToCall);
114117
}
115118
}
116119

117120
/**
118121
* Calls lifecycle hooks with their contexts, skipping init hooks if it's not
119-
* creation mode.
122+
* the first LView pass.
120123
*
121124
* @param currentView The current view
122125
* @param arr The array in which the hooks are found

0 commit comments

Comments
 (0)