Skip to content

Commit ca11ef2

Browse files
mheveryalxhub
mhevery
authored and
alxhub
committed
fix(core): Store ICU state in LView rather than in TView (angular#39233)
Before this refactoring/fix the ICU would store the current selected index in `TView`. This is incorrect, since if ICU is in `ngFor` it will cause issues in some circumstances. This refactoring properly moves the state to `LView`. closes angular#37021 closes angular#38144 closes angular#38073 PR Close angular#39233
1 parent 6790848 commit ca11ef2

53 files changed

Lines changed: 3262 additions & 1635 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎goldens/circular-deps/packages.json‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@
223223
"packages/core/src/render3/assert.ts",
224224
"packages/core/src/render3/interfaces/container.ts",
225225
"packages/core/src/render3/interfaces/node.ts",
226+
"packages/core/src/render3/interfaces/i18n.ts",
226227
"packages/core/src/render3/interfaces/view.ts",
227228
"packages/core/src/di/injector.ts",
228229
"packages/core/src/di/r3_injector.ts",
@@ -239,6 +240,7 @@
239240
"packages/core/src/render3/assert.ts",
240241
"packages/core/src/render3/interfaces/container.ts",
241242
"packages/core/src/render3/interfaces/node.ts",
243+
"packages/core/src/render3/interfaces/i18n.ts",
242244
"packages/core/src/render3/interfaces/view.ts",
243245
"packages/core/src/metadata.ts",
244246
"packages/core/src/di.ts",
@@ -262,6 +264,7 @@
262264
"packages/core/src/render3/assert.ts",
263265
"packages/core/src/render3/interfaces/container.ts",
264266
"packages/core/src/render3/interfaces/node.ts",
267+
"packages/core/src/render3/interfaces/i18n.ts",
265268
"packages/core/src/render3/interfaces/view.ts",
266269
"packages/core/src/render3/interfaces/definition.ts",
267270
"packages/core/src/core.ts",
@@ -968,25 +971,37 @@
968971
[
969972
"packages/core/src/render3/interfaces/container.ts",
970973
"packages/core/src/render3/interfaces/node.ts",
974+
"packages/core/src/render3/interfaces/i18n.ts",
971975
"packages/core/src/render3/interfaces/view.ts"
972976
],
973977
[
974978
"packages/core/src/render3/interfaces/definition.ts",
975979
"packages/core/src/render3/interfaces/node.ts",
980+
"packages/core/src/render3/interfaces/i18n.ts",
976981
"packages/core/src/render3/interfaces/view.ts"
977982
],
978983
[
979984
"packages/core/src/render3/interfaces/definition.ts",
980985
"packages/core/src/render3/interfaces/view.ts"
981986
],
982987
[
983-
"packages/core/src/render3/interfaces/node.ts",
988+
"packages/core/src/render3/interfaces/i18n.ts",
989+
"packages/core/src/render3/interfaces/node.ts"
990+
],
991+
[
992+
"packages/core/src/render3/interfaces/i18n.ts",
984993
"packages/core/src/render3/interfaces/view.ts"
985994
],
986995
[
987-
"packages/core/src/render3/interfaces/node.ts",
996+
"packages/core/src/render3/interfaces/i18n.ts",
988997
"packages/core/src/render3/interfaces/view.ts",
989-
"packages/core/src/render3/interfaces/query.ts"
998+
"packages/core/src/render3/interfaces/node.ts"
999+
],
1000+
[
1001+
"packages/core/src/render3/interfaces/i18n.ts",
1002+
"packages/core/src/render3/interfaces/view.ts",
1003+
"packages/core/src/render3/interfaces/query.ts",
1004+
"packages/core/src/render3/interfaces/node.ts"
9901005
],
9911006
[
9921007
"packages/core/src/render3/interfaces/query.ts",

‎goldens/public-api/core/core.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,7 @@ export declare abstract class Renderer2 {
799799
abstract createElement(name: string, namespace?: string | null): any;
800800
abstract createText(value: string): any;
801801
abstract destroy(): void;
802-
abstract insertBefore(parent: any, newChild: any, refChild: any): void;
802+
abstract insertBefore(parent: any, newChild: any, refChild: any, isMove?: boolean): void;
803803
abstract listen(target: 'window' | 'document' | 'body' | any, eventName: string, callback: (event: any) => boolean | void): () => void;
804804
abstract nextSibling(node: any): any;
805805
abstract parentNode(node: any): any;

‎goldens/size-tracking/aio-payloads.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"master": {
1313
"uncompressed": {
1414
"runtime-es2015": 3037,
15-
"main-es2015": 447742,
15+
"main-es2015": 448676,
1616
"polyfills-es2015": 52415
1717
}
1818
}

‎goldens/size-tracking/integration-payloads.json‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"master": {
44
"uncompressed": {
55
"runtime-es2015": 1485,
6-
"main-es2015": 140199,
6+
"main-es2015": 140899,
77
"polyfills-es2015": 36571
88
}
99
}
@@ -12,7 +12,7 @@
1212
"master": {
1313
"uncompressed": {
1414
"runtime-es2015": 1485,
15-
"main-es2015": 16650,
15+
"main-es2015": 17092,
1616
"polyfills-es2015": 36657
1717
}
1818
}
@@ -21,7 +21,7 @@
2121
"master": {
2222
"uncompressed": {
2323
"runtime-es2015": 1485,
24-
"main-es2015": 146417,
24+
"main-es2015": 147242,
2525
"polyfills-es2015": 36571
2626
}
2727
}
@@ -30,7 +30,7 @@
3030
"master": {
3131
"uncompressed": {
3232
"runtime-es2015": 1485,
33-
"main-es2015": 135003,
33+
"main-es2015": 136096,
3434
"polyfills-es2015": 37248
3535
}
3636
}
@@ -39,7 +39,7 @@
3939
"master": {
4040
"uncompressed": {
4141
"runtime-es2015": 2289,
42-
"main-es2015": 241850,
42+
"main-es2015": 242460,
4343
"polyfills-es2015": 36938,
4444
"5-es2015": 751
4545
}
@@ -49,7 +49,7 @@
4949
"master": {
5050
"uncompressed": {
5151
"runtime-es2015": 2289,
52-
"main-es2015": 217827,
52+
"main-es2015": 218527,
5353
"polyfills-es2015": 36723,
5454
"5-es2015": 781
5555
}

‎packages/core/src/render/api.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,13 @@ export abstract class Renderer2 {
158158
* @param parent The parent node.
159159
* @param newChild The new child nodes.
160160
* @param refChild The existing child node before which `newChild` is inserted.
161+
* @param isMove Optional argument which signifies if the current `insertBefore` is a result of a
162+
* move. Animation uses this information to trigger move animations. In the past the Animation
163+
* would always assume that any `insertBefore` is a move. This is not strictly true because
164+
* with runtime i18n it is possible to invoke `insertBefore` as a result of i18n and it should
165+
* not trigger an animation move.
161166
*/
162-
abstract insertBefore(parent: any, newChild: any, refChild: any): void;
167+
abstract insertBefore(parent: any, newChild: any, refChild: any, isMove?: boolean): void;
163168
/**
164169
* Implement this callback to remove a child node from the host element's DOM.
165170
* @param parent The parent node.

‎packages/core/src/render3/assert.ts‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {assertDefined, assertEqual, assertNumber, throwError} from '../util/asse
1010
import {getComponentDef, getNgModuleDef} from './definition';
1111
import {LContainer} from './interfaces/container';
1212
import {DirectiveDef} from './interfaces/definition';
13+
import {TIcu} from './interfaces/i18n';
1314
import {NodeInjectorOffset} from './interfaces/injector';
1415
import {TNode} from './interfaces/node';
1516
import {isLContainer, isLView} from './interfaces/type_checks';
@@ -25,13 +26,28 @@ export function assertTNodeForLView(tNode: TNode, lView: LView) {
2526
}
2627

2728
export function assertTNodeForTView(tNode: TNode, tView: TView) {
28-
assertDefined(tNode, 'TNode must be defined');
29+
assertTNode(tNode);
2930
tNode.hasOwnProperty('tView_') &&
3031
assertEqual(
3132
(tNode as any as {tView_: TView}).tView_, tView,
3233
'This TNode does not belong to this TView.');
3334
}
3435

36+
export function assertTNode(tNode: TNode) {
37+
assertDefined(tNode, 'TNode must be defined');
38+
if (!(tNode && typeof tNode === 'object' && tNode.hasOwnProperty('directiveStylingLast'))) {
39+
throwError('Not of type TNode, got: ' + tNode);
40+
}
41+
}
42+
43+
44+
export function assertTIcu(tIcu: TIcu) {
45+
assertDefined(tIcu, 'Expected TIcu to be defined');
46+
if (!(typeof tIcu.currentCaseLViewIndex === 'number')) {
47+
throwError('Object is not of TIcu type.');
48+
}
49+
}
50+
3551
export function assertComponentType(
3652
actual: any,
3753
msg: string = 'Type passed in is not ComponentType, it does not have \'ɵcmp\' property.') {
@@ -106,18 +122,15 @@ export function assertIndexInDeclRange(lView: LView, index: number) {
106122
export function assertIndexInVarsRange(lView: LView, index: number) {
107123
const tView = lView[1];
108124
assertBetween(
109-
tView.bindingStartIndex, (tView as any as {i18nStartIndex: number}).i18nStartIndex, index);
110-
}
111-
112-
export function assertIndexInI18nRange(lView: LView, index: number) {
113-
const tView = lView[1];
114-
assertBetween(
115-
(tView as any as {i18nStartIndex: number}).i18nStartIndex, tView.expandoStartIndex, index);
125+
tView.bindingStartIndex,
126+
(tView as any as {originalExpandoStartIndex: number}).originalExpandoStartIndex, index);
116127
}
117128

118129
export function assertIndexInExpandoRange(lView: LView, index: number) {
119130
const tView = lView[1];
120-
assertBetween(tView.expandoStartIndex, lView.length, index);
131+
assertBetween(
132+
(tView as any as {originalExpandoStartIndex: number}).originalExpandoStartIndex, lView.length,
133+
index);
121134
}
122135

123136
export function assertBetween(lower: number, upper: number, index: number) {

‎packages/core/src/render3/component.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ export function renderComponent<T>(
163163
* @param rNode Render host element.
164164
* @param def ComponentDef
165165
* @param rootView The parent view where the host node is stored
166+
* @param rendererFactory Factory to be used for creating child renderers.
166167
* @param hostRenderer The current renderer
167168
* @param sanitizer The sanitizer, if provided
168169
*
@@ -174,7 +175,10 @@ export function createRootComponentView(
174175
const tView = rootView[TVIEW];
175176
ngDevMode && assertIndexInRange(rootView, 0 + HEADER_OFFSET);
176177
rootView[0 + HEADER_OFFSET] = rNode;
177-
const tNode: TElementNode = getOrCreateTNode(tView, 0, TNodeType.Element, null, null);
178+
// '#host' is added here as we don't know the real host DOM name (we don't want to read it) and at
179+
// the same time we want to communicate the the debug `TNode` that this is a special `TNode`
180+
// representing a host element.
181+
const tNode = getOrCreateTNode(tView, 0, TNodeType.Element, '#host', null);
178182
const mergedAttrs = tNode.mergedAttrs = def.hostAttrs;
179183
if (mergedAttrs !== null) {
180184
computeStaticStyling(tNode, mergedAttrs, true);

‎packages/core/src/render3/component_ref.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ import {assertComponentType} from './assert';
2323
import {createRootComponent, createRootComponentView, createRootContext, LifecycleHooksFeature} from './component';
2424
import {getComponentDef} from './definition';
2525
import {NodeInjector} from './di';
26-
import {createLView, createTView, elementCreate, locateHostElement, renderView} from './instructions/shared';
26+
import {createLView, createTView, locateHostElement, renderView} from './instructions/shared';
2727
import {ComponentDef} from './interfaces/definition';
2828
import {TContainerNode, TElementContainerNode, TElementNode, TNode} from './interfaces/node';
2929
import {domRendererFactory3, RendererFactory3, RNode} from './interfaces/renderer';
3030
import {LView, LViewFlags, TViewType} from './interfaces/view';
3131
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
32-
import {writeDirectClass} from './node_manipulation';
32+
import {createElementNode, writeDirectClass} from './node_manipulation';
3333
import {extractAttrsAndClassesFromSelector, stringifyCSSSelectorList} from './node_selector_matcher';
3434
import {enterView, leaveView} from './state';
3535
import {setUpAttributes} from './util/attrs_utils';
@@ -147,8 +147,8 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
147147
const elementName = this.componentDef.selectors[0][0] as string || 'div';
148148
const hostRNode = rootSelectorOrNode ?
149149
locateHostElement(hostRenderer, rootSelectorOrNode, this.componentDef.encapsulation) :
150-
elementCreate(
151-
elementName, rendererFactory.createRenderer(null, this.componentDef),
150+
createElementNode(
151+
rendererFactory.createRenderer(null, this.componentDef), elementName,
152152
getNamespace(elementName));
153153

154154
const rootFlags = this.componentDef.onPush ? LViewFlags.Dirty | LViewFlags.IsRoot :

‎packages/core/src/render3/context_discovery.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,7 @@ function findViaNativeElement(lView: LView, target: RElement): number {
204204
* Locates the next tNode (child, sibling or parent).
205205
*/
206206
function traverseNextElement(tNode: TNode): TNode|null {
207-
if (tNode.child && tNode.child.parent === tNode) {
208-
// FIXME(misko): checking if `tNode.child.parent === tNode` should not be necessary
209-
// We have added it here because i18n creates TNode's which are not valid, so this is a work
210-
// around. The i18n code will be refactored in #39003 and once it lands this extra check can be
211-
// deleted.
207+
if (tNode.child) {
212208
return tNode.child;
213209
} else if (tNode.next) {
214210
return tNode.next;

‎packages/core/src/render3/i18n/i18n.md‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,6 @@ The i18n markers are:
115115
- `index`: the index of the `template` instruction, as defined in the template instructions (e.g. `template(index, ...)`).
116116
- `block`: the index of the parent sub-template block, in which this child sub-template block was declared.
117117

118-
- `�!{index}:{block}�/�/!{index}:{block}�`: *Projection block*: Marks the beginning and end of <ng-content> that was embedded in the original translation block.
119-
- `index`: the index of the projection, as defined in the template instructions (e.g. `projection(index, ...)`).
120-
- `block` (*optional*): the index of the parent sub-template block, in which this child sub-template block was declared.
121-
122118
No other i18n marker format is supported.
123119

124120
The i18n markers in the example above can be interpreted as follows:

0 commit comments

Comments
 (0)