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

refactor(ivy): remove pChild from LNode #24705

Closed
wants to merge 1 commit into from
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
27 changes: 18 additions & 9 deletions packages/core/src/render3/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
*/

import {assertEqual, assertLessThan} from './assert';
import {NO_CHANGE, bindingUpdated, createLNode, getPreviousOrParentNode, getRenderer, getViewData, load} from './instructions';
import {NO_CHANGE, bindingUpdated, createLNode, getPreviousOrParentNode, getRenderer, getViewData, load, resetApplicationState} from './instructions';
import {RENDER_PARENT} from './interfaces/container';
import {LContainerNode, LElementNode, LNode, TNodeType} from './interfaces/node';
import {BINDING_INDEX} from './interfaces/view';
import {BINDING_INDEX, HEADER_OFFSET, TVIEW} from './interfaces/view';
import {appendChild, createTextNode, getParentLNode, removeChild} from './node_manipulation';
import {stringify} from './util';

Expand Down Expand Up @@ -240,10 +240,16 @@ function appendI18nNode(node: LNode, parentNode: LNode, previousNode: LNode) {

appendChild(parentNode, node.native || null, viewData);

if (previousNode === parentNode && parentNode.pChild === null) {
parentNode.pChild = node;
} else {
previousNode.pNextOrParent = node;
// On first pass, re-organize node tree to put this node in the correct position.
if (node.view[TVIEW].firstTemplatePass) {
node.tNode.next = null;
if (previousNode === parentNode && node.tNode !== parentNode.tNode.child) {
node.tNode.next = parentNode.tNode.child;
parentNode.tNode.child = node.tNode;
} else if (previousNode !== parentNode && node.tNode !== previousNode.tNode.next) {
node.tNode.next = previousNode.tNode.next;
previousNode.tNode.next = node.tNode;
}
}

// Template containers also have a comment node for the `ViewContainerRef` that should be moved
Expand Down Expand Up @@ -278,6 +284,7 @@ export function i18nApply(startIndex: number, instructions: I18nInstruction[]):
const renderer = getRenderer();
let localParentNode: LNode = getParentLNode(load(startIndex)) || getPreviousOrParentNode();
let localPreviousNode: LNode = localParentNode;
resetApplicationState(); // We don't want to add to the tree with the wrong previous node

for (let i = 0; i < instructions.length; i++) {
const instruction = instructions[i] as number;
Expand All @@ -298,10 +305,12 @@ export function i18nApply(startIndex: number, instructions: I18nInstruction[]):
const value = instructions[++i];
const textRNode = createTextNode(value, renderer);
// If we were to only create a `RNode` then projections won't move the text.
// But since this text doesn't have an index in `LViewData`, we need to create an
// `LElementNode` with the index -1 so that it isn't saved in `LViewData`
const textLNode = createLNode(-1, TNodeType.Element, textRNode, null, null);
// Create text node at the current end of viewData. Must subtract header offset because
// createLNode takes a raw index (not adjusted by header offset).
const textLNode =
createLNode(viewData.length - HEADER_OFFSET, TNodeType.Element, textRNode, null, null);
localPreviousNode = appendI18nNode(textLNode, localParentNode, localPreviousNode);
resetApplicationState();
break;
case I18nInstructions.CloseNode:
localPreviousNode = localParentNode;
Expand Down
22 changes: 4 additions & 18 deletions packages/core/src/render3/instructions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,7 @@ export function createLNodeObject(
queries: queries,
tNode: null !,
pNextOrParent: null,
dynamicLContainerNode: null,
pChild: null,
dynamicLContainerNode: null
};
}

Expand Down Expand Up @@ -440,7 +439,7 @@ export function createLNode(
/**
* Resets the application state.
*/
function resetApplicationState() {
export function resetApplicationState() {
isParent = false;
previousOrParentNode = null !;
}
Expand Down Expand Up @@ -1894,16 +1893,7 @@ export function projectionDef(
}

const componentNode: LElementNode = findComponentHost(viewData);
let isProjectingI18nNodes = false;
let componentChild: LNode|null;
// for i18n translations we use pChild to point to the next child
// TODO(kara): Remove when removing LNodes
if (componentNode.pChild) {
isProjectingI18nNodes = true;
componentChild = componentNode.pChild;
} else {
componentChild = getChildLNode(componentNode);
}
let componentChild = getChildLNode(componentNode);

while (componentChild !== null) {
// execute selector matching logic if and only if:
Expand All @@ -1916,11 +1906,7 @@ export function projectionDef(
distributedNodes[0].push(componentChild);
}

if (isProjectingI18nNodes) {
componentChild = componentChild.pNextOrParent;
} else {
componentChild = getNextLNode(componentChild);
}
componentChild = getNextLNode(componentChild);
}

ngDevMode && assertDataNext(index + HEADER_OFFSET);
Expand Down
7 changes: 0 additions & 7 deletions packages/core/src/render3/interfaces/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,6 @@ export interface LNode {
*/
pNextOrParent: LNode|null;

/**
* If this node is part of an i18n block, pointer to the first child after translation
* If this node is not part of an i18n block, this field is null.
*/
// TODO(kara): Remove this when removing pNextOrParent
pChild: LNode|null;

/**
* Pointer to the corresponding TNode object, which stores static
* data about this node.
Expand Down
3 changes: 0 additions & 3 deletions packages/core/src/render3/node_manipulation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ export function getNextLNode(node: LNode): LNode|null {

/** Retrieves the first child of a given node */
export function getChildLNode(node: LNode): LNode|null {
if (node.pChild) {
return node.pChild;
}
if (node.tNode.child) {
const viewData = node.tNode.type === TNodeType.View ? node.data as LViewData : node.view;
return viewData[node.tNode.child.index];
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/render3/i18n_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ describe('Runtime i18n', () => {

// Translated to:
// <ul i18n>
// <li *ngFor="let item of items">valeur: {{item}}</li>
// <li *ngFor="let item of items">valeur: {{item}}!</li>
// </ul>
template: (rf: RenderFlags, myApp: MyApp) => {
if (rf & RenderFlags.Create) {
Expand Down