Skip to content

Commit

Permalink
refactor(core): Improve tree shakability of i18n code. (#39301)
Browse files Browse the repository at this point in the history
`TNode.insertBeforeIndex` is only populated when i18n is present. This
change puts all code which reads `insertBeforeIndex` behind a
dynamically loaded functions which are set only when i18n code executes.

PR Close #39301
  • Loading branch information
mhevery authored and alxhub committed Oct 22, 2020
1 parent ffd4161 commit d939b5f
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 59 deletions.
4 changes: 2 additions & 2 deletions goldens/size-tracking/integration-payloads.json
Expand Up @@ -12,7 +12,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 1485,
"main-es2015": 17092,
"main-es2015": 17049,
"polyfills-es2015": 36657
}
}
Expand Down Expand Up @@ -49,7 +49,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2289,
"main-es2015": 218527,
"main-es2015": 217879,
"polyfills-es2015": 36723,
"5-es2015": 781
}
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/render3/i18n/i18n_insert_before_index.ts
Expand Up @@ -8,6 +8,8 @@

import {assertEqual} from '../../util/assert';
import {TNode, TNodeType} from '../interfaces/node';
import {setI18nHandling} from '../node_manipulation';
import {getInsertInFrontOfRNodeWithI18n, processI18nInsertBefore} from '../node_manipulation_i18n';

/**
* Add `tNode` to `previousTNodes` list and update relevant `TNode`s in `previousTNodes` list
Expand Down Expand Up @@ -81,6 +83,7 @@ function setInsertBeforeIndex(tNode: TNode, value: number): void {
// Array is stored if we have to insert child nodes. See `TNode.insertBeforeIndex`
index[0] = value;
} else {
setI18nHandling(getInsertInFrontOfRNodeWithI18n, processI18nInsertBefore);
tNode.insertBeforeIndex = value;
}
}
2 changes: 1 addition & 1 deletion packages/core/src/render3/i18n/i18n_parse.ts
Expand Up @@ -117,7 +117,7 @@ export function i18nStartFirstCreatePass(
}
} else {
// Odd indexes are placeholders (elements and sub-templates)
// At this point value is something like: '/#1:2' (orginally coming from '�/#1:2�')
// At this point value is something like: '/#1:2' (originally coming from '�/#1:2�')
const isClosing = value.charCodeAt(0) === CharCode.SLASH;
const type = value.charCodeAt(isClosing ? 1 : 0);
ngDevMode && assertOneOf(type, CharCode.STAR, CharCode.HASH, CharCode.EXCLAMATION);
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/render3/i18n/i18n_util.ts
Expand Up @@ -13,6 +13,8 @@ import {IcuCreateOpCode, TIcu} from '../interfaces/i18n';
import {TIcuContainerNode, TNode, TNodeType} from '../interfaces/node';
import {LView, TView} from '../interfaces/view';
import {assertTNodeType} from '../node_assert';
import {setI18nHandling} from '../node_manipulation';
import {getInsertInFrontOfRNodeWithI18n, processI18nInsertBefore} from '../node_manipulation_i18n';
import {addTNodeAndUpdateInsertBeforeIndex} from './i18n_insert_before_index';


Expand Down Expand Up @@ -83,6 +85,7 @@ export function setTNodeInsertBeforeIndex(tNode: TNode, index: number) {
ngDevMode && assertTNode(tNode);
let insertBeforeIndex = tNode.insertBeforeIndex;
if (insertBeforeIndex === null) {
setI18nHandling(getInsertInFrontOfRNodeWithI18n, processI18nInsertBefore);
insertBeforeIndex = tNode.insertBeforeIndex =
[null!/* may be updated to number later */, index];
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/instructions/shared.ts
Expand Up @@ -12,7 +12,7 @@ import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from '../../me
import {ViewEncapsulation} from '../../metadata/view';
import {validateAgainstEventAttributes, validateAgainstEventProperties} from '../../sanitization/sanitization';
import {Sanitizer} from '../../sanitization/sanitizer';
import {assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertGreaterThanOrEqual, assertIndexInRange, assertNotEqual, assertNotSame, assertSame, assertString} from '../../util/assert';
import {assertDefined, assertDomNode, assertEqual, assertGreaterThanOrEqual, assertIndexInRange, assertNotEqual, assertNotSame, assertSame, assertString} from '../../util/assert';
import {createNamedArrayType} from '../../util/named_array_type';
import {initNgDevMode} from '../../util/ng_dev_mode';
import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect';
Expand Down
104 changes: 49 additions & 55 deletions packages/core/src/render3/node_manipulation.ts
Expand Up @@ -639,28 +639,64 @@ export function nativeNextSibling(renderer: Renderer3, node: RNode): RNode|null
* Find a node in front of which `currentTNode` should be inserted.
*
* This method determines the `RNode` in front of which we should insert the `currentRNode`. This
* takes `TNode.insertBeforeIndex` into account.
* takes `TNode.insertBeforeIndex` into account if i18n code has been invoked.
*
* @param parentTNode parent `TNode`
* @param currentTNode current `TNode` (The node which we would like to insert into the DOM)
* @param lView current `LView`
*/
function getInsertInFrontOfRNode(parentTNode: TNode, currentTNode: TNode, lView: LView): RNode|
null {
const tNodeInsertBeforeIndex = currentTNode.insertBeforeIndex;
const insertBeforeIndex =
Array.isArray(tNodeInsertBeforeIndex) ? tNodeInsertBeforeIndex[0] : tNodeInsertBeforeIndex;
if (insertBeforeIndex === null) {
if (parentTNode.type & (TNodeType.ElementContainer | TNodeType.Icu)) {
return getNativeByTNode(parentTNode, lView);
}
} else {
ngDevMode && assertIndexInRange(lView, insertBeforeIndex);
return unwrapRNode(lView[insertBeforeIndex]);
return _getInsertInFrontOfRNodeWithI18n(parentTNode, currentTNode, lView);
}


/**
* Find a node in front of which `currentTNode` should be inserted. (Does not take i18n into
* account)
*
* This method determines the `RNode` in front of which we should insert the `currentRNode`. This
* does not take `TNode.insertBeforeIndex` into account.
*
* @param parentTNode parent `TNode`
* @param currentTNode current `TNode` (The node which we would like to insert into the DOM)
* @param lView current `LView`
*/
export function getInsertInFrontOfRNodeWithNoI18n(
parentTNode: TNode, currentTNode: TNode, lView: LView): RNode|null {
if (parentTNode.type & (TNodeType.ElementContainer | TNodeType.Icu)) {
return getNativeByTNode(parentTNode, lView);
}
return null;
}

/**
* Tree shakable boundary for `getInsertInFrontOfRNodeWithI18n` function.
*
* This function will only be set if i18n code runs.
*/
let _getInsertInFrontOfRNodeWithI18n: (parentTNode: TNode, currentTNode: TNode, lView: LView) =>
RNode | null = getInsertInFrontOfRNodeWithNoI18n;

/**
* Tree shakable boundary for `processI18nInsertBefore` function.
*
* This function will only be set if i18n code runs.
*/
let _processI18nInsertBefore: (
renderer: Renderer3, childTNode: TNode, lView: LView, childRNode: RNode|RNode[],
parentRElement: RElement|null) => void;

export function setI18nHandling(
getInsertInFrontOfRNodeWithI18n: (parentTNode: TNode, currentTNode: TNode, lView: LView) =>
RNode | null,
processI18nInsertBefore: (
renderer: Renderer3, childTNode: TNode, lView: LView, childRNode: RNode|RNode[],
parentRElement: RElement|null) => void) {
_getInsertInFrontOfRNodeWithI18n = getInsertInFrontOfRNodeWithI18n;
_processI18nInsertBefore = processI18nInsertBefore;
}

/**
* Appends the `child` native node (or a collection of nodes) to the `parent`.
*
Expand All @@ -685,50 +721,8 @@ export function appendChild(
}
}

const tNodeInsertBeforeIndex = childTNode.insertBeforeIndex;
if (Array.isArray(tNodeInsertBeforeIndex) &&
(childTNode.flags & TNodeFlags.isComponentHost) === 0) {
// An array indicates that there are i18n nodes that need to be added as children of this
// `rChildNode`. These i18n nodes were created before this `rChildNode` was available and so
// only now can be added. The first element of the array is the normal index where we should
// insert the `rChildNode`. Additional elements are the extra nodes to be added as children of
// `rChildNode`.
processI18nText(renderer, childTNode, lView, childRNode, parentRNode, tNodeInsertBeforeIndex);
}
}

/**
* Process `TNode.insertBeforeIndex` by adding i18n text nodes.
*
* See `TNode.insertBeforeIndex`
*
* @param renderer
* @param childTNode
* @param lView
* @param childRNode
* @param parentRElement
* @param i18nChildren
*/
function processI18nText(
renderer: Renderer3, childTNode: TNode, lView: LView, childRNode: RNode|RNode[],
parentRElement: RElement|null, i18nChildren: number[]): void {
ngDevMode && assertDomNode(childRNode);
const isProcedural = isProceduralRenderer(renderer);
let i18nParent: RElement|null = childRNode as RElement;
let anchorRNode: RNode|null = null;
if (!(childTNode.type & TNodeType.AnyRNode)) {
anchorRNode = i18nParent;
i18nParent = parentRElement;
}
const isViewRoot = childTNode.parent === null;
if (i18nParent !== null) {
for (let i = 1; i < i18nChildren.length; i++) {
// No need to `unwrapRNode` because all of the indexes point to i18n text nodes.
// see `assertDomNode` below.
const i18nChild = lView[i18nChildren[i]];
nativeInsertBefore(renderer, i18nParent, i18nChild, anchorRNode, false);
}
}
_processI18nInsertBefore !== undefined &&
_processI18nInsertBefore(renderer, childTNode, lView, childRNode, parentRNode);
}

/**
Expand Down
72 changes: 72 additions & 0 deletions packages/core/src/render3/node_manipulation_i18n.ts
@@ -0,0 +1,72 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {assertDomNode, assertIndexInRange} from '../util/assert';
import {TNode, TNodeFlags, TNodeType} from './interfaces/node';
import {RElement, Renderer3, RNode} from './interfaces/renderer';
import {LView} from './interfaces/view';
import {getInsertInFrontOfRNodeWithNoI18n, nativeInsertBefore} from './node_manipulation';
import {unwrapRNode} from './util/view_utils';


/**
* Find a node in front of which `currentTNode` should be inserted (takes i18n into account).
*
* This method determines the `RNode` in front of which we should insert the `currentRNode`. This
* takes `TNode.insertBeforeIndex` into account.
*
* @param parentTNode parent `TNode`
* @param currentTNode current `TNode` (The node which we would like to insert into the DOM)
* @param lView current `LView`
*/
export function getInsertInFrontOfRNodeWithI18n(
parentTNode: TNode, currentTNode: TNode, lView: LView): RNode|null {
const tNodeInsertBeforeIndex = currentTNode.insertBeforeIndex;
const insertBeforeIndex =
Array.isArray(tNodeInsertBeforeIndex) ? tNodeInsertBeforeIndex[0] : tNodeInsertBeforeIndex;
if (insertBeforeIndex === null) {
return getInsertInFrontOfRNodeWithNoI18n(parentTNode, currentTNode, lView);
} else {
ngDevMode && assertIndexInRange(lView, insertBeforeIndex);
return unwrapRNode(lView[insertBeforeIndex]);
}
}


/**
* Process `TNode.insertBeforeIndex` by adding i18n text nodes.
*
* See `TNode.insertBeforeIndex`
*/
export function processI18nInsertBefore(
renderer: Renderer3, childTNode: TNode, lView: LView, childRNode: RNode|RNode[],
parentRElement: RElement|null): void {
const tNodeInsertBeforeIndex = childTNode.insertBeforeIndex;
if (Array.isArray(tNodeInsertBeforeIndex)) {
// An array indicates that there are i18n nodes that need to be added as children of this
// `childRNode`. These i18n nodes were created before this `childRNode` was available and so
// only now can be added. The first element of the array is the normal index where we should
// insert the `childRNode`. Additional elements are the extra nodes to be added as children of
// `childRNode`.
ngDevMode && assertDomNode(childRNode);
let i18nParent: RElement|null = childRNode as RElement;
let anchorRNode: RNode|null = null;
if (!(childTNode.type & TNodeType.AnyRNode)) {
anchorRNode = i18nParent;
i18nParent = parentRElement;
}
if (i18nParent !== null && (childTNode.flags & TNodeFlags.isComponentHost) === 0) {
for (let i = 1; i < tNodeInsertBeforeIndex.length; i++) {
// No need to `unwrapRNode` because all of the indexes point to i18n text nodes.
// see `assertDomNode` below.
const i18nChild = lView[tNodeInsertBeforeIndex[i]];
nativeInsertBefore(renderer, i18nParent, i18nChild, anchorRNode, false);
}
}
}
}
Expand Up @@ -232,5 +232,8 @@
},
{
"name": "viewAttachedToChangeDetector"
},
{
"name": "ɵɵtext"
}
]

0 comments on commit d939b5f

Please sign in to comment.