fix(ivy): re-create node links after creating/moving/removing nodes with i18n (#28827)

I18n can change the order of the nodes, or insert new dynamic nodes. When that happens it can break the existing links (`TNode.next`) or even create loops:
```
div1 → div2 → div3 → div4 → div5
```

Can become:
```
div1 → div4 → div2 → div3  div5
         🡑             │
         └─────────────┘
```

This PR fixes this issue by recreating the broken links between those nodes.
PR Close #28827
This commit is contained in:
Olivier Combe 2019-02-19 16:02:28 +01:00 committed by Igor Minar
parent 3c1a1620e3
commit ad6475ffac
2 changed files with 80 additions and 9 deletions

View File

@ -10,13 +10,12 @@ import {SRCSET_ATTRS, URI_ATTRS, VALID_ATTRS, VALID_ELEMENTS, getTemplateContent
import {InertBodyHelper} from '../sanitization/inert_body'; import {InertBodyHelper} from '../sanitization/inert_body';
import {_sanitizeUrl, sanitizeSrcset} from '../sanitization/url_sanitizer'; import {_sanitizeUrl, sanitizeSrcset} from '../sanitization/url_sanitizer';
import {assertDefined, assertEqual, assertGreaterThan} from '../util/assert'; import {assertDefined, assertEqual, assertGreaterThan} from '../util/assert';
import {attachPatchData} from './context_discovery'; import {attachPatchData} from './context_discovery';
import {allocExpando, createNodeAtIndex, elementAttribute, load, textBinding} from './instructions'; import {allocExpando, createNodeAtIndex, elementAttribute, load, textBinding} from './instructions';
import {LContainer, NATIVE} from './interfaces/container'; import {LContainer, NATIVE} from './interfaces/container';
import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, IcuType, TI18n, TIcu} from './interfaces/i18n'; import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, IcuType, TI18n, TIcu} from './interfaces/i18n';
import {TElementNode, TIcuContainerNode, TNode, TNodeType} from './interfaces/node'; import {TElementNode, TIcuContainerNode, TNode, TNodeType} from './interfaces/node';
import {RComment, RElement} from './interfaces/renderer'; import {RComment, RElement, RText} from './interfaces/renderer';
import {SanitizerFn} from './interfaces/sanitization'; import {SanitizerFn} from './interfaces/sanitization';
import {StylingContext} from './interfaces/styling'; import {StylingContext} from './interfaces/styling';
import {BINDING_INDEX, HEADER_OFFSET, LView, RENDERER, TVIEW, TView, T_HOST} from './interfaces/view'; import {BINDING_INDEX, HEADER_OFFSET, LView, RENDERER, TVIEW, TView, T_HOST} from './interfaces/view';
@ -466,11 +465,13 @@ function i18nStartFirstPass(
function appendI18nNode(tNode: TNode, parentTNode: TNode, previousTNode: TNode | null): TNode { function appendI18nNode(tNode: TNode, parentTNode: TNode, previousTNode: TNode | null): TNode {
ngDevMode && ngDevMode.rendererMoveNode++; ngDevMode && ngDevMode.rendererMoveNode++;
const nextNode = tNode.next;
const viewData = getLView(); const viewData = getLView();
if (!previousTNode) { if (!previousTNode) {
previousTNode = parentTNode; previousTNode = parentTNode;
} }
// re-organize node tree to put this node in the correct position.
// Re-organize node tree to put this node in the correct position.
if (previousTNode === parentTNode && tNode !== parentTNode.child) { if (previousTNode === parentTNode && tNode !== parentTNode.child) {
tNode.next = parentTNode.child; tNode.next = parentTNode.child;
parentTNode.child = tNode; parentTNode.child = tNode;
@ -485,6 +486,15 @@ function appendI18nNode(tNode: TNode, parentTNode: TNode, previousTNode: TNode |
tNode.parent = parentTNode as TElementNode; tNode.parent = parentTNode as TElementNode;
} }
// If tNode was moved around, we might need to fix a broken link.
let cursor: TNode|null = tNode.next;
while (cursor) {
if (cursor.next === tNode) {
cursor.next = nextNode;
}
cursor = cursor.next;
}
appendChild(getNativeByTNode(tNode, viewData), tNode, viewData); appendChild(getNativeByTNode(tNode, viewData), tNode, viewData);
const slotValue = viewData[tNode.index]; const slotValue = viewData[tNode.index];
@ -655,6 +665,24 @@ function i18nEndFirstPass(tView: TView) {
} }
} }
/**
* Creates and stores the dynamic TNode, and unhooks it from the tree for now.
*/
function createDynamicNodeAtIndex(
index: number, type: TNodeType, native: RElement | RText | null,
name: string | null): TElementNode|TIcuContainerNode {
const previousOrParentTNode = getPreviousOrParentTNode();
const tNode = createNodeAtIndex(index, type as any, native, name, null);
// We are creating a dynamic node, the previous tNode might not be pointing at this node.
// We will link ourselves into the tree later with `appendI18nNode`.
if (previousOrParentTNode.next === tNode) {
previousOrParentTNode.next = null;
}
return tNode;
}
function readCreateOpCodes( function readCreateOpCodes(
index: number, createOpCodes: I18nMutateOpCodes, icus: TIcu[] | null, index: number, createOpCodes: I18nMutateOpCodes, icus: TIcu[] | null,
viewData: LView): number[] { viewData: LView): number[] {
@ -669,7 +697,7 @@ function readCreateOpCodes(
const textNodeIndex = createOpCodes[++i] as number; const textNodeIndex = createOpCodes[++i] as number;
ngDevMode && ngDevMode.rendererCreateTextNode++; ngDevMode && ngDevMode.rendererCreateTextNode++;
previousTNode = currentTNode; previousTNode = currentTNode;
currentTNode = createNodeAtIndex(textNodeIndex, TNodeType.Element, textRNode, null, null); currentTNode = createDynamicNodeAtIndex(textNodeIndex, TNodeType.Element, textRNode, null);
visitedNodes.push(textNodeIndex); visitedNodes.push(textNodeIndex);
setIsParent(false); setIsParent(false);
} else if (typeof opCode == 'number') { } else if (typeof opCode == 'number') {
@ -729,8 +757,8 @@ function readCreateOpCodes(
const commentRNode = renderer.createComment(commentValue); const commentRNode = renderer.createComment(commentValue);
ngDevMode && ngDevMode.rendererCreateComment++; ngDevMode && ngDevMode.rendererCreateComment++;
previousTNode = currentTNode; previousTNode = currentTNode;
currentTNode = currentTNode = createDynamicNodeAtIndex(
createNodeAtIndex(commentNodeIndex, TNodeType.IcuContainer, commentRNode, null, null); commentNodeIndex, TNodeType.IcuContainer, commentRNode, null);
visitedNodes.push(commentNodeIndex); visitedNodes.push(commentNodeIndex);
attachPatchData(commentRNode, viewData); attachPatchData(commentRNode, viewData);
(currentTNode as TIcuContainerNode).activeCaseIndex = null; (currentTNode as TIcuContainerNode).activeCaseIndex = null;
@ -746,8 +774,8 @@ function readCreateOpCodes(
const elementRNode = renderer.createElement(tagNameValue); const elementRNode = renderer.createElement(tagNameValue);
ngDevMode && ngDevMode.rendererCreateElement++; ngDevMode && ngDevMode.rendererCreateElement++;
previousTNode = currentTNode; previousTNode = currentTNode;
currentTNode = createNodeAtIndex( currentTNode = createDynamicNodeAtIndex(
elementNodeIndex, TNodeType.Element, elementRNode, tagNameValue, null); elementNodeIndex, TNodeType.Element, elementRNode, tagNameValue);
visitedNodes.push(elementNodeIndex); visitedNodes.push(elementNodeIndex);
break; break;
default: default:

View File

@ -12,7 +12,7 @@ import {defineComponent, defineDirective} from '../../src/render3/definition';
import {getTranslationForTemplate, i18n, i18nApply, i18nAttributes, i18nEnd, i18nExp, i18nPostprocess, i18nStart} from '../../src/render3/i18n'; import {getTranslationForTemplate, i18n, i18nApply, i18nAttributes, i18nEnd, i18nExp, i18nPostprocess, i18nStart} from '../../src/render3/i18n';
import {RenderFlags} from '../../src/render3/interfaces/definition'; import {RenderFlags} from '../../src/render3/interfaces/definition';
import {AttributeMarker} from '../../src/render3/interfaces/node'; import {AttributeMarker} from '../../src/render3/interfaces/node';
import {getNativeByIndex} from '../../src/render3/util'; import {getNativeByIndex, getTNode} from '../../src/render3/util';
import {NgIf} from './common_with_def'; import {NgIf} from './common_with_def';
import {allocHostVars, element, elementEnd, elementStart, template, text, nextContext, bind, elementProperty, projectionDef, projection, elementContainerStart, elementContainerEnd} from '../../src/render3/instructions'; import {allocHostVars, element, elementEnd, elementStart, template, text, nextContext, bind, elementProperty, projectionDef, projection, elementContainerStart, elementContainerEnd} from '../../src/render3/instructions';
import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nUpdateOpCode, I18nUpdateOpCodes, TI18n} from '../../src/render3/interfaces/i18n'; import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nUpdateOpCode, I18nUpdateOpCodes, TI18n} from '../../src/render3/interfaces/i18n';
@ -1436,6 +1436,49 @@ describe('Runtime i18n', () => {
`<div class="bar" title="start 3 middle 2 end">trad 2 emails<!--ICU 23--></div><div class="bar"></div>`); `<div class="bar" title="start 3 middle 2 end">trad 2 emails<!--ICU 23--></div><div class="bar"></div>`);
}); });
it('should fix the links when adding/moving/removing nodes', () => {
const MSG_DIV = `<EFBFBD>#2<><32>/#2<><32>#8<><38>/#8<><38>#4<><34>/#4<><34>#5<><35>/#5<>Hello World<6C>#3<><33>/#3<><33>#7<><37>/#7<>`;
let fixture = prepareFixture(() => {
elementStart(0, 'div');
{
i18nStart(1, MSG_DIV);
{
element(2, 'div2');
element(3, 'div3');
element(4, 'div4');
element(5, 'div5');
element(6, 'div6');
element(7, 'div7');
element(8, 'div8');
}
i18nEnd();
}
elementEnd();
}, null, 9);
expect(fixture.html)
.toEqual(
'<div><div2></div2><div8></div8><div4></div4><div5></div5>Hello World<div3></div3><div7></div7></div>');
const div0 = getTNode(0, fixture.hostView);
const div2 = getTNode(2, fixture.hostView);
const div3 = getTNode(3, fixture.hostView);
const div4 = getTNode(4, fixture.hostView);
const div5 = getTNode(5, fixture.hostView);
const div7 = getTNode(7, fixture.hostView);
const div8 = getTNode(8, fixture.hostView);
const text = getTNode(9, fixture.hostView);
expect(div0.child).toEqual(div2);
expect(div0.next).toBeNull();
expect(div2.next).toEqual(div8);
expect(div8.next).toEqual(div4);
expect(div4.next).toEqual(div5);
expect(div5.next).toEqual(text);
expect(text.next).toEqual(div3);
expect(div3.next).toEqual(div7);
expect(div7.next).toBeNull();
});
describe('projection', () => { describe('projection', () => {
it('should project the translations', () => { it('should project the translations', () => {
@Component({selector: 'child', template: '<p><ng-content></ng-content></p>'}) @Component({selector: 'child', template: '<p><ng-content></ng-content></p>'})