From 6d8c73a4d62380bd7cfcaeaa23d5db802467d485 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Tue, 4 Aug 2020 12:42:12 -0700 Subject: [PATCH] fix(core): Store the currently selected ICU in `LView` (#38345) The currently selected ICU was incorrectly being stored it `TNode` rather than in `LView`. Remove: `TIcuContainerNode.activeCaseIndex` Add: `LView[TIcu.currentCaseIndex]` PR Close #38345 --- .../size-tracking/integration-payloads.json | 2 +- packages/core/src/render3/bindings.ts | 4 +- packages/core/src/render3/component.ts | 4 +- packages/core/src/render3/i18n.ts | 194 +++++++----- .../core/src/render3/instructions/advance.ts | 4 +- .../core/src/render3/instructions/element.ts | 4 +- .../render3/instructions/element_container.ts | 4 +- .../core/src/render3/instructions/listener.ts | 4 +- .../src/render3/instructions/lview_debug.ts | 25 +- .../core/src/render3/instructions/shared.ts | 232 ++++++++------- .../core/src/render3/instructions/text.ts | 5 +- packages/core/src/render3/interfaces/i18n.ts | 8 + packages/core/src/render3/interfaces/node.ts | 7 - packages/core/src/render3/pure_function.ts | 4 +- packages/core/src/render3/query.ts | 8 +- .../src/render3/styling/style_binding_list.ts | 4 +- packages/core/src/render3/util/view_utils.ts | 8 +- packages/core/src/util/assert.ts | 2 +- packages/core/test/acceptance/i18n_spec.ts | 275 +++++++++++++++++- packages/core/test/render3/i18n_spec.ts | 109 +++---- 20 files changed, 624 insertions(+), 283 deletions(-) diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index 9a036944cf..19530230df 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -62,7 +62,7 @@ "bundle": "TODO(i): we should define ngDevMode to false in Closure, but --define only works in the global scope.", "bundle": "TODO(i): (FW-2164) TS 3.9 new class shape seems to have broken Closure in big ways. The size went from 169991 to 252338", "bundle": "TODO(i): after removal of tsickle from ngc-wrapped / ng_package, we had to switch to SIMPLE optimizations which increased the size from 252338 to 1198917, see PR#37221 and PR#37317 for more info", - "bundle": 1213130 + "bundle": 1213769 } } } diff --git a/packages/core/src/render3/bindings.ts b/packages/core/src/render3/bindings.ts index 333bff96d6..0968e054ad 100644 --- a/packages/core/src/render3/bindings.ts +++ b/packages/core/src/render3/bindings.ts @@ -7,7 +7,7 @@ */ import {devModeEqual} from '../change_detection/change_detection_util'; -import {assertDataInRange, assertLessThan, assertNotSame} from '../util/assert'; +import {assertIndexInRange, assertLessThan, assertNotSame} from '../util/assert'; import {getExpressionChangedErrorDetails, throwErrorIfNoChangesMode} from './errors'; import {LView} from './interfaces/view'; @@ -24,7 +24,7 @@ export function updateBinding(lView: LView, bindingIndex: number, value: any): a /** Gets the current binding value. */ export function getBinding(lView: LView, bindingIndex: number): any { - ngDevMode && assertDataInRange(lView, bindingIndex); + ngDevMode && assertIndexInRange(lView, bindingIndex); ngDevMode && assertNotSame(lView[bindingIndex], NO_CHANGE, 'Stored value should never be NO_CHANGE.'); return lView[bindingIndex]; diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index 3bd154914f..0f411079a1 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -11,7 +11,7 @@ import {Type} from '../core'; import {Injector} from '../di/injector'; import {Sanitizer} from '../sanitization/sanitizer'; -import {assertDataInRange} from '../util/assert'; +import {assertIndexInRange} from '../util/assert'; import {assertComponentType} from './assert'; import {getComponentDef} from './definition'; @@ -172,7 +172,7 @@ export function createRootComponentView( rNode: RElement|null, def: ComponentDef, rootView: LView, rendererFactory: RendererFactory3, hostRenderer: Renderer3, sanitizer?: Sanitizer|null): LView { const tView = rootView[TVIEW]; - ngDevMode && assertDataInRange(rootView, 0 + HEADER_OFFSET); + ngDevMode && assertIndexInRange(rootView, 0 + HEADER_OFFSET); rootView[0 + HEADER_OFFSET] = rNode; const tNode: TElementNode = getOrCreateTNode(tView, null, 0, TNodeType.Element, null, null); const mergedAttrs = tNode.mergedAttrs = def.hostAttrs; diff --git a/packages/core/src/render3/i18n.ts b/packages/core/src/render3/i18n.ts index ab8e7b4dfc..e7da73a264 100644 --- a/packages/core/src/render3/i18n.ts +++ b/packages/core/src/render3/i18n.ts @@ -13,13 +13,13 @@ import {getTemplateContent, SRCSET_ATTRS, URI_ATTRS, VALID_ATTRS, VALID_ELEMENTS import {getInertBodyHelper} from '../sanitization/inert_body'; import {_sanitizeUrl, sanitizeSrcset} from '../sanitization/url_sanitizer'; import {addAllToArray} from '../util/array_utils'; -import {assertDataInRange, assertDefined, assertEqual} from '../util/assert'; +import {assertDefined, assertEqual, assertGreaterThan, assertIndexInRange} from '../util/assert'; import {bindingUpdated} from './bindings'; import {attachPatchData} from './context_discovery'; import {i18nMutateOpCodesToString, i18nUpdateOpCodesToString} from './i18n_debug'; import {setDelayProjection} from './instructions/all'; -import {allocExpando, elementAttributeInternal, elementPropertyInternal, getOrCreateTNode, setInputsForProperty, setNgReflectProperties, textBindingInternal} from './instructions/shared'; +import {allocExpando, elementAttributeInternal, elementPropertyInternal, getOrCreateTNode, setInputsForProperty, setNgReflectProperties, textBindingInternal as applyTextBinding} from './instructions/shared'; import {LContainer, NATIVE} from './interfaces/container'; import {getDocument} from './interfaces/document'; import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, IcuType, TI18n, TIcu} from './interfaces/i18n'; @@ -726,7 +726,7 @@ function i18nEndFirstPass(tView: TView, lView: LView) { const lastCreatedNode = getPreviousOrParentTNode(); // Read the instructions to insert/move/remove DOM elements - const visitedNodes = readCreateOpCodes(rootIndex, tI18n.create, tView, lView); + const visitedNodes = applyCreateOpCodes(tView, rootIndex, tI18n.create, lView); // Remove deleted nodes let index = rootIndex + 1; @@ -756,7 +756,7 @@ function createDynamicNodeAtIndex( tView: TView, lView: LView, index: number, type: TNodeType, native: RElement|RText|null, name: string|null): TElementNode|TIcuContainerNode { const previousOrParentTNode = getPreviousOrParentTNode(); - ngDevMode && assertDataInRange(lView, index + HEADER_OFFSET); + ngDevMode && assertIndexInRange(lView, index + HEADER_OFFSET); lView[index + HEADER_OFFSET] = native; // FIXME(misko): Why does this create A TNode??? I would not expect this to be here. const tNode = getOrCreateTNode(tView, lView[T_HOST], index, type as any, name, null); @@ -770,8 +770,16 @@ function createDynamicNodeAtIndex( return tNode; } -function readCreateOpCodes( - index: number, createOpCodes: I18nMutateOpCodes, tView: TView, lView: LView): number[] { +/** + * Apply `I18nMutateOpCodes` OpCodes. + * + * @param tView Current `TView` + * @param rootIndex Pointer to the root (parent) tNode for the i18n. + * @param createOpCodes OpCodes to process + * @param lView Current `LView` + */ +function applyCreateOpCodes( + tView: TView, rootindex: number, createOpCodes: I18nMutateOpCodes, lView: LView): number[] { const renderer = lView[RENDERER]; let currentTNode: TNode|null = null; let previousTNode: TNode|null = null; @@ -792,7 +800,7 @@ function readCreateOpCodes( case I18nMutateOpCode.AppendChild: const destinationNodeIndex = opCode >>> I18nMutateOpCode.SHIFT_PARENT; let destinationTNode: TNode; - if (destinationNodeIndex === index) { + if (destinationNodeIndex === rootindex) { // If the destination node is `i18nStart`, we don't have a // top-level node and we should use the host node instead destinationTNode = lView[T_HOST]!; @@ -852,7 +860,6 @@ function readCreateOpCodes( tView, lView, commentNodeIndex, TNodeType.IcuContainer, commentRNode, null); visitedNodes.push(commentNodeIndex); attachPatchData(commentRNode, lView); - (currentTNode as TIcuContainerNode).activeCaseIndex = null; // We will add the case nodes later, during the update phase setIsNotParent(); break; @@ -881,16 +888,27 @@ function readCreateOpCodes( return visitedNodes; } -function readUpdateOpCodes( - updateOpCodes: I18nUpdateOpCodes, icus: TIcu[]|null, bindingsStartIndex: number, - changeMask: number, tView: TView, lView: LView, bypassCheckBit: boolean) { +/** + * Apply `I18nUpdateOpCodes` OpCodes + * + * @param tView Current `TView` + * @param tIcus If ICUs present than this contains them. + * @param lView Current `LView` + * @param updateOpCodes OpCodes to process + * @param bindingsStartIndex Location of the first `ɵɵi18nApply` + * @param changeMask Each bit corresponds to a `ɵɵi18nExp` (Counting backwards from + * `bindingsStartIndex`) + */ +function applyUpdateOpCodes( + tView: TView, tIcus: TIcu[]|null, lView: LView, updateOpCodes: I18nUpdateOpCodes, + bindingsStartIndex: number, changeMask: number) { let caseCreated = false; for (let i = 0; i < updateOpCodes.length; i++) { // bit code to check if we should apply the next update const checkBit = updateOpCodes[i] as number; // Number of opCodes to skip until next set of update codes const skipCodes = updateOpCodes[++i] as number; - if (bypassCheckBit || (checkBit & changeMask)) { + if (checkBit & changeMask) { // The value has been updated since last checked let value = ''; for (let j = i + 1; j <= (i + skipCodes); j++) { @@ -912,16 +930,16 @@ function readUpdateOpCodes( sanitizeFn, false); break; case I18nUpdateOpCode.Text: - textBindingInternal(lView, nodeIndex, value); + applyTextBinding(lView, nodeIndex, value); break; case I18nUpdateOpCode.IcuSwitch: - caseCreated = icuSwitchCase( - tView, updateOpCodes[++j] as number, nodeIndex, icus!, lView, value); + caseCreated = + applyIcuSwitchCase(tView, tIcus!, updateOpCodes[++j] as number, lView, value); break; case I18nUpdateOpCode.IcuUpdate: - icuUpdateCase( - tView, lView, updateOpCodes[++j] as number, nodeIndex, bindingsStartIndex, - icus!, caseCreated); + applyIcuUpdateCase( + tView, tIcus!, updateOpCodes[++j] as number, bindingsStartIndex, lView, + caseCreated); break; } } @@ -932,68 +950,100 @@ function readUpdateOpCodes( } } -function icuUpdateCase( - tView: TView, lView: LView, tIcuIndex: number, nodeIndex: number, bindingsStartIndex: number, - tIcus: TIcu[], caseCreated: boolean) { +/** + * Apply OpCodes associated with updating an existing ICU. + * + * @param tView Current `TView` + * @param tIcus tIcus ICUs active at this location them. + * @param tIcuIndex Index into `tIcus` to process. + * @param bindingsStartIndex Location of the first `ɵɵi18nApply` + * @param lView Current `LView` + * @param changeMask Each bit corresponds to a `ɵɵi18nExp` (Counting backwards from + * `bindingsStartIndex`) + */ +function applyIcuUpdateCase( + tView: TView, tIcus: TIcu[], tIcuIndex: number, bindingsStartIndex: number, lView: LView, + caseCreated: boolean) { + ngDevMode && assertIndexInRange(tIcus, tIcuIndex); const tIcu = tIcus[tIcuIndex]; - const icuTNode = getTNode(tView, nodeIndex) as TIcuContainerNode; - if (icuTNode.activeCaseIndex !== null) { - readUpdateOpCodes( - tIcu.update[icuTNode.activeCaseIndex], tIcus, bindingsStartIndex, changeMask, tView, lView, - caseCreated); + ngDevMode && assertIndexInRange(lView, tIcu.currentCaseLViewIndex); + const activeCaseIndex = lView[tIcu.currentCaseLViewIndex]; + if (activeCaseIndex !== null) { + const mask = caseCreated ? + -1 : // -1 is same as all bits on, which simulates creation since it marks all bits dirty + changeMask; + applyUpdateOpCodes(tView, tIcus, lView, tIcu.update[activeCaseIndex], bindingsStartIndex, mask); } } -function icuSwitchCase( - tView: TView, tIcuIndex: number, nodeIndex: number, tIcus: TIcu[], lView: LView, - value: string): boolean { - const tIcu = tIcus[tIcuIndex]; - const icuTNode = getTNode(tView, nodeIndex) as TIcuContainerNode; +/** + * Apply OpCodes associated with switching a case on ICU. + * + * This involves tearing down existing case and than building up a new case. + * + * @param tView Current `TView` + * @param tIcus ICUs active at this location. + * @param tIcuIndex Index into `tIcus` to process. + * @param lView Current `LView` + * @param value Value of the case to update to. + * @returns true if a new case was created (needed so that the update executes regardless of the + * bitmask) + */ +function applyIcuSwitchCase( + tView: TView, tIcus: TIcu[], tIcuIndex: number, lView: LView, value: string): boolean { + applyIcuSwitchCaseRemove(tView, tIcus, tIcuIndex, lView); + + // Rebuild a new case for this ICU let caseCreated = false; - // If there is an active case, delete the old nodes - if (icuTNode.activeCaseIndex !== null) { - const removeCodes = tIcu.remove[icuTNode.activeCaseIndex]; + ngDevMode && assertIndexInRange(tIcus, tIcuIndex); + const tIcu = tIcus[tIcuIndex]; + const caseIndex = getCaseIndex(tIcu, value); + lView[tIcu.currentCaseLViewIndex] = caseIndex !== -1 ? caseIndex : null; + if (caseIndex > -1) { + // Add the nodes for the new case + applyCreateOpCodes( + tView, -1, // -1 means we don't have parent node + tIcu.create[caseIndex], lView); + caseCreated = true; + } + return caseCreated; +} + +/** + * Apply OpCodes associated with tearing down of DOM. + * + * This involves tearing down existing case and than building up a new case. + * + * @param tView Current `TView` + * @param tIcus ICUs active at this location. + * @param tIcuIndex Index into `tIcus` to process. + * @param lView Current `LView` + * @returns true if a new case was created (needed so that the update executes regardless of the + * bitmask) + */ +function applyIcuSwitchCaseRemove(tView: TView, tIcus: TIcu[], tIcuIndex: number, lView: LView) { + ngDevMode && assertIndexInRange(tIcus, tIcuIndex); + const tIcu = tIcus[tIcuIndex]; + const activeCaseIndex = lView[tIcu.currentCaseLViewIndex]; + if (activeCaseIndex !== null) { + const removeCodes = tIcu.remove[activeCaseIndex]; for (let k = 0; k < removeCodes.length; k++) { const removeOpCode = removeCodes[k] as number; const nodeOrIcuIndex = removeOpCode >>> I18nMutateOpCode.SHIFT_REF; switch (removeOpCode & I18nMutateOpCode.MASK_INSTRUCTION) { case I18nMutateOpCode.Remove: + // FIXME(misko): this comment is wrong! // Remove DOM element, but do *not* mark TNode as detached, since we are // just switching ICU cases (while keeping the same TNode), so a DOM element // representing a new ICU case will be re-created. removeNode(tView, lView, nodeOrIcuIndex, /* markAsDetached */ false); break; case I18nMutateOpCode.RemoveNestedIcu: - removeNestedIcu( - tView, tIcus, removeCodes, nodeOrIcuIndex, - removeCodes[k + 1] as number >>> I18nMutateOpCode.SHIFT_REF); + applyIcuSwitchCaseRemove(tView, tIcus, nodeOrIcuIndex, lView); break; } } } - - // Update the active caseIndex - const caseIndex = getCaseIndex(tIcu, value); - icuTNode.activeCaseIndex = caseIndex !== -1 ? caseIndex : null; - if (caseIndex > -1) { - // Add the nodes for the new case - readCreateOpCodes( - -1 /* -1 means we don't have parent node */, tIcu.create[caseIndex], tView, lView); - caseCreated = true; - } - return caseCreated; -} - -function removeNestedIcu( - tView: TView, tIcus: TIcu[], removeCodes: I18nMutateOpCodes, nodeIndex: number, - nestedIcuNodeIndex: number) { - const nestedIcuTNode = getTNode(tView, nestedIcuNodeIndex) as TIcuContainerNode; - const activeIndex = nestedIcuTNode.activeCaseIndex; - if (activeIndex !== null) { - const nestedTIcu = tIcus[nodeIndex]; - // FIXME(misko): the fact that we are adding items to parent list looks very suspect! - addAllToArray(nestedTIcu.remove[activeIndex], removeCodes); - } } function removeNode(tView: TView, lView: LView, index: number, markAsDetached: boolean) { @@ -1153,18 +1203,18 @@ export function ɵɵi18nApply(index: number) { if (shiftsCounter) { const tView = getTView(); ngDevMode && assertDefined(tView, `tView should be defined`); - const tI18n = tView.data[index + HEADER_OFFSET]; + const tI18n = tView.data[index + HEADER_OFFSET] as TI18n | I18nUpdateOpCodes; let updateOpCodes: I18nUpdateOpCodes; - let icus: TIcu[]|null = null; + let tIcus: TIcu[]|null = null; if (Array.isArray(tI18n)) { updateOpCodes = tI18n as I18nUpdateOpCodes; } else { updateOpCodes = (tI18n as TI18n).update; - icus = (tI18n as TI18n).icus; + tIcus = (tI18n as TI18n).icus; } const bindingsStartIndex = getBindingIndex() - shiftsCounter - 1; const lView = getLView(); - readUpdateOpCodes(updateOpCodes, icus, bindingsStartIndex, changeMask, tView, lView, false); + applyUpdateOpCodes(tView, tIcus, lView, updateOpCodes, bindingsStartIndex, changeMask); // Reset changeMask & maskBit to default for the next update cycle changeMask = 0b0; @@ -1215,9 +1265,10 @@ function icuStart( const updateCodes: I18nUpdateOpCodes[] = []; const vars = []; const childIcus: number[][] = []; - for (let i = 0; i < icuExpression.values.length; i++) { + const values = icuExpression.values; + for (let i = 0; i < values.length; i++) { // Each value is an array of strings & other ICU expressions - const valueArr = icuExpression.values[i]; + const valueArr = values[i]; const nestedIcus: IcuExpression[] = []; for (let j = 0; j < valueArr.length; j++) { const value = valueArr[j]; @@ -1239,6 +1290,9 @@ function icuStart( const tIcu: TIcu = { type: icuExpression.type, vars, + currentCaseLViewIndex: HEADER_OFFSET + + expandoStartIndex // expandoStartIndex does not include the header so add it. + + 1, // The first item stored is the `` anchor so skip it. childIcus, cases: icuExpression.cases, create: createCodes, @@ -1269,7 +1323,13 @@ function parseIcuCase( throw new Error('Unable to generate inert body element'); } const wrapper = getTemplateContent(inertBodyElement!) as Element || inertBodyElement; - const opCodes: IcuCase = {vars: 0, childIcus: [], create: [], remove: [], update: []}; + const opCodes: IcuCase = { + vars: 1, // allocate space for `TIcu.currentCaseLViewIndex` + childIcus: [], + create: [], + remove: [], + update: [] + }; if (ngDevMode) { attachDebugGetter(opCodes.create, i18nMutateOpCodesToString); attachDebugGetter(opCodes.remove, i18nMutateOpCodesToString); diff --git a/packages/core/src/render3/instructions/advance.ts b/packages/core/src/render3/instructions/advance.ts index 0abc852850..3eb585b93c 100644 --- a/packages/core/src/render3/instructions/advance.ts +++ b/packages/core/src/render3/instructions/advance.ts @@ -5,7 +5,7 @@ * 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 {assertDataInRange, assertGreaterThan} from '../../util/assert'; +import {assertGreaterThan, assertIndexInRange} from '../../util/assert'; import {executeCheckHooks, executeInitAndCheckHooks} from '../hooks'; import {FLAGS, HEADER_OFFSET, InitPhaseState, LView, LViewFlags, TView} from '../interfaces/view'; import {getCheckNoChangesMode, getLView, getSelectedIndex, getTView, setSelectedIndex} from '../state'; @@ -52,7 +52,7 @@ export function ɵɵselect(index: number): void { export function selectIndexInternal( tView: TView, lView: LView, index: number, checkNoChangesMode: boolean) { ngDevMode && assertGreaterThan(index, -1, 'Invalid index'); - ngDevMode && assertDataInRange(lView, index + HEADER_OFFSET); + ngDevMode && assertIndexInRange(lView, index + HEADER_OFFSET); // Flush the initial hooks for elements in the view that have been added up to this point. // PERF WARNING: do NOT extract this to a separate function without running benchmarks diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts index 2b9b82c73f..aaa2f8e4fe 100644 --- a/packages/core/src/render3/instructions/element.ts +++ b/packages/core/src/render3/instructions/element.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {assertDataInRange, assertDefined, assertEqual} from '../../util/assert'; +import {assertDefined, assertEqual, assertIndexInRange} from '../../util/assert'; import {assertFirstCreatePass, assertHasParent} from '../assert'; import {attachPatchData} from '../context_discovery'; import {registerPostOrderHooks} from '../hooks'; @@ -79,7 +79,7 @@ export function ɵɵelementStart( getBindingIndex(), tView.bindingStartIndex, 'elements should be created before any bindings'); ngDevMode && ngDevMode.rendererCreateElement++; - ngDevMode && assertDataInRange(lView, adjustedIndex); + ngDevMode && assertIndexInRange(lView, adjustedIndex); const renderer = lView[RENDERER]; const native = lView[adjustedIndex] = elementCreate(name, renderer, getNamespace()); diff --git a/packages/core/src/render3/instructions/element_container.ts b/packages/core/src/render3/instructions/element_container.ts index 05d5b57052..6a7a6ef7a4 100644 --- a/packages/core/src/render3/instructions/element_container.ts +++ b/packages/core/src/render3/instructions/element_container.ts @@ -5,7 +5,7 @@ * 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 {assertDataInRange, assertEqual} from '../../util/assert'; +import {assertEqual, assertIndexInRange} from '../../util/assert'; import {assertHasParent} from '../assert'; import {attachPatchData} from '../context_discovery'; import {registerPostOrderHooks} from '../hooks'; @@ -66,7 +66,7 @@ export function ɵɵelementContainerStart( const tView = getTView(); const adjustedIndex = index + HEADER_OFFSET; - ngDevMode && assertDataInRange(lView, adjustedIndex); + ngDevMode && assertIndexInRange(lView, adjustedIndex); ngDevMode && assertEqual( getBindingIndex(), tView.bindingStartIndex, diff --git a/packages/core/src/render3/instructions/listener.ts b/packages/core/src/render3/instructions/listener.ts index 4caf2af8e1..37b8766724 100644 --- a/packages/core/src/render3/instructions/listener.ts +++ b/packages/core/src/render3/instructions/listener.ts @@ -7,7 +7,7 @@ */ -import {assertDataInRange} from '../../util/assert'; +import {assertIndexInRange} from '../../util/assert'; import {isObservable} from '../../util/lang'; import {EMPTY_OBJ} from '../empty'; import {PropertyAliasValue, TNode, TNodeFlags, TNodeType} from '../interfaces/node'; @@ -204,7 +204,7 @@ function listenerInternal( if (propsLength) { for (let i = 0; i < propsLength; i += 2) { const index = props[i] as number; - ngDevMode && assertDataInRange(lView, index); + ngDevMode && assertIndexInRange(lView, index); const minifiedName = props[i + 1]; const directiveInstance = lView[index]; const output = directiveInstance[minifiedName]; diff --git a/packages/core/src/render3/instructions/lview_debug.ts b/packages/core/src/render3/instructions/lview_debug.ts index 7ea4307567..27bf8eeb9c 100644 --- a/packages/core/src/render3/instructions/lview_debug.ts +++ b/packages/core/src/render3/instructions/lview_debug.ts @@ -362,19 +362,24 @@ export function toDebug(obj: any): any { * (will not serialize child elements). */ function toHtml(value: any, includeChildren: boolean = false): string|null { - const node: HTMLElement|null = unwrapRNode(value) as any; + const node: Node|null = unwrapRNode(value) as any; if (node) { - const isTextNode = node.nodeType === Node.TEXT_NODE; - const outerHTML = (isTextNode ? node.textContent : node.outerHTML) || ''; - if (includeChildren || isTextNode) { - return outerHTML; - } else { - const innerHTML = '>' + node.innerHTML + '<'; - return (outerHTML.split(innerHTML)[0]) + '>'; + switch (node.nodeType) { + case Node.TEXT_NODE: + return node.textContent; + case Node.COMMENT_NODE: + return ``; + case Node.ELEMENT_NODE: + const outerHTML = (node as Element).outerHTML; + if (includeChildren) { + return outerHTML; + } else { + const innerHTML = '>' + (node as Element).innerHTML + '<'; + return (outerHTML.split(innerHTML)[0]) + '>'; + } } - } else { - return null; } + return null; } export class LViewDebug implements ILViewDebug { diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 929a5fa78c..c707856b0f 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -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 {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertLessThan, assertNotEqual, assertNotSame, assertSame} from '../../util/assert'; +import {assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertIndexInRange, assertLessThan, assertNotEqual, assertNotSame, assertSame} from '../../util/assert'; import {createNamedArrayType} from '../../util/named_array_type'; import {initNgDevMode} from '../../util/ng_dev_mode'; import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect'; @@ -236,13 +236,6 @@ export function getOrCreateTNode( const tNode = tView.data[adjustedIndex] as TNode || createTNodeAtIndex(tView, tHostNode, adjustedIndex, type, name, attrs); setPreviousOrParentTNode(tNode, true); - if (ngDevMode) { - // For performance reasons it is important that the tNode retains the same shape during runtime. - // (To make sure that all of the code is monomorphic.) For this reason we seal the object to - // prevent class transitions. - // FIXME(misko): re-enable this once i18n code is compliant with this. - // Object.seal(tNode); - } return tNode as TElementNode & TViewNode & TContainerNode & TElementContainerNode & TProjectionNode & TIcuContainerNode; } @@ -665,44 +658,44 @@ export function createTView( // that has a host binding, we will update the blueprint with that def's hostVars count. const initialViewLength = bindingStartIndex + vars; const blueprint = createViewBlueprint(bindingStartIndex, initialViewLength); - return blueprint[TVIEW as any] = ngDevMode ? + const tView = blueprint[TVIEW as any] = ngDevMode ? new TViewConstructor( - type, - viewIndex, // id: number, - blueprint, // blueprint: LView, - templateFn, // template: ComponentTemplate<{}>|null, - null, // queries: TQueries|null - viewQuery, // viewQuery: ViewQueriesFunction<{}>|null, - null!, // node: TViewNode|TElementNode|null, - cloneToTViewData(blueprint).fill(null, bindingStartIndex), // data: TData, - bindingStartIndex, // bindingStartIndex: number, - initialViewLength, // expandoStartIndex: number, - null, // expandoInstructions: ExpandoInstructions|null, - true, // firstCreatePass: boolean, - true, // firstUpdatePass: boolean, - false, // staticViewQueries: boolean, - false, // staticContentQueries: boolean, - null, // preOrderHooks: HookData|null, - null, // preOrderCheckHooks: HookData|null, - null, // contentHooks: HookData|null, - null, // contentCheckHooks: HookData|null, - null, // viewHooks: HookData|null, - null, // viewCheckHooks: HookData|null, - null, // destroyHooks: DestroyHookData|null, - null, // cleanup: any[]|null, - null, // contentQueries: number[]|null, - null, // components: number[]|null, - typeof directives === 'function' ? - directives() : - directives, // directiveRegistry: DirectiveDefList|null, - typeof pipes === 'function' ? pipes() : pipes, // pipeRegistry: PipeDefList|null, - null, // firstChild: TNode|null, - schemas, // schemas: SchemaMetadata[]|null, - consts, // consts: TConstants|null - false, // incompleteFirstPass: boolean - decls, // ngDevMode only: decls - vars, // ngDevMode only: vars - ) : + type, + viewIndex, // id: number, + blueprint, // blueprint: LView, + templateFn, // template: ComponentTemplate<{}>|null, + null, // queries: TQueries|null + viewQuery, // viewQuery: ViewQueriesFunction<{}>|null, + null!, // node: TViewNode|TElementNode|null, + cloneToTViewData(blueprint).fill(null, bindingStartIndex), // data: TData, + bindingStartIndex, // bindingStartIndex: number, + initialViewLength, // expandoStartIndex: number, + null, // expandoInstructions: ExpandoInstructions|null, + true, // firstCreatePass: boolean, + true, // firstUpdatePass: boolean, + false, // staticViewQueries: boolean, + false, // staticContentQueries: boolean, + null, // preOrderHooks: HookData|null, + null, // preOrderCheckHooks: HookData|null, + null, // contentHooks: HookData|null, + null, // contentCheckHooks: HookData|null, + null, // viewHooks: HookData|null, + null, // viewCheckHooks: HookData|null, + null, // destroyHooks: DestroyHookData|null, + null, // cleanup: any[]|null, + null, // contentQueries: number[]|null, + null, // components: number[]|null, + typeof directives === 'function' ? + directives() : + directives, // directiveRegistry: DirectiveDefList|null, + typeof pipes === 'function' ? pipes() : pipes, // pipeRegistry: PipeDefList|null, + null, // firstChild: TNode|null, + schemas, // schemas: SchemaMetadata[]|null, + consts, // consts: TConstants|null + false, // incompleteFirstPass: boolean + decls, // ngDevMode only: decls + vars, // ngDevMode only: vars + ) : { type: type, id: viewIndex, @@ -736,6 +729,13 @@ export function createTView( consts: consts, incompleteFirstPass: false }; + if (ngDevMode) { + // For performance reasons it is important that the tView retains the same shape during runtime. + // (To make sure that all of the code is monomorphic.) For this reason we seal the object to + // prevent class transitions. + Object.seal(tView); + } + return tView; } function createViewBlueprint(bindingStartIndex: number, initialViewLength: number): LView { @@ -825,71 +825,79 @@ export function createTNode( tagName: string|null, attrs: TAttributes|null): TNode { ngDevMode && ngDevMode.tNode++; let injectorIndex = tParent ? tParent.injectorIndex : -1; - return ngDevMode ? new TNodeDebug( - tView, // tView_: TView - type, // type: TNodeType - adjustedIndex, // index: number - injectorIndex, // injectorIndex: number - -1, // directiveStart: number - -1, // directiveEnd: number - -1, // directiveStylingLast: number - null, // propertyBindings: number[]|null - 0, // flags: TNodeFlags - 0, // providerIndexes: TNodeProviderIndexes - tagName, // tagName: string|null - attrs, // attrs: (string|AttributeMarker|(string|SelectorFlags)[])[]|null - null, // mergedAttrs - null, // localNames: (string|number)[]|null - undefined, // initialInputs: (string[]|null)[]|null|undefined - null, // inputs: PropertyAliases|null - null, // outputs: PropertyAliases|null - null, // tViews: ITView|ITView[]|null - null, // next: ITNode|null - null, // projectionNext: ITNode|null - null, // child: ITNode|null - tParent, // parent: TElementNode|TContainerNode|null - null, // projection: number|(ITNode|RNode[])[]|null - null, // styles: string|null - null, // stylesWithoutHost: string|null - undefined, // residualStyles: string|null - null, // classes: string|null - null, // classesWithoutHost: string|null - undefined, // residualClasses: string|null - 0 as any, // classBindings: TStylingRange; - 0 as any, // styleBindings: TStylingRange; - ) : - { - type: type, - index: adjustedIndex, - injectorIndex: injectorIndex, - directiveStart: -1, - directiveEnd: -1, - directiveStylingLast: -1, - propertyBindings: null, - flags: 0, - providerIndexes: 0, - tagName: tagName, - attrs: attrs, - mergedAttrs: null, - localNames: null, - initialInputs: undefined, - inputs: null, - outputs: null, - tViews: null, - next: null, - projectionNext: null, - child: null, - parent: tParent, - projection: null, - styles: null, - stylesWithoutHost: null, - residualStyles: undefined, - classes: null, - classesWithoutHost: null, - residualClasses: undefined, - classBindings: 0 as any, - styleBindings: 0 as any, - }; + const tNode = ngDevMode ? + new TNodeDebug( + tView, // tView_: TView + type, // type: TNodeType + adjustedIndex, // index: number + injectorIndex, // injectorIndex: number + -1, // directiveStart: number + -1, // directiveEnd: number + -1, // directiveStylingLast: number + null, // propertyBindings: number[]|null + 0, // flags: TNodeFlags + 0, // providerIndexes: TNodeProviderIndexes + tagName, // tagName: string|null + attrs, // attrs: (string|AttributeMarker|(string|SelectorFlags)[])[]|null + null, // mergedAttrs + null, // localNames: (string|number)[]|null + undefined, // initialInputs: (string[]|null)[]|null|undefined + null, // inputs: PropertyAliases|null + null, // outputs: PropertyAliases|null + null, // tViews: ITView|ITView[]|null + null, // next: ITNode|null + null, // projectionNext: ITNode|null + null, // child: ITNode|null + tParent, // parent: TElementNode|TContainerNode|null + null, // projection: number|(ITNode|RNode[])[]|null + null, // styles: string|null + null, // stylesWithoutHost: string|null + undefined, // residualStyles: string|null + null, // classes: string|null + null, // classesWithoutHost: string|null + undefined, // residualClasses: string|null + 0 as any, // classBindings: TStylingRange; + 0 as any, // styleBindings: TStylingRange; + ) : + { + type: type, + index: adjustedIndex, + injectorIndex: injectorIndex, + directiveStart: -1, + directiveEnd: -1, + directiveStylingLast: -1, + propertyBindings: null, + flags: 0, + providerIndexes: 0, + tagName: tagName, + attrs: attrs, + mergedAttrs: null, + localNames: null, + initialInputs: undefined, + inputs: null, + outputs: null, + tViews: null, + next: null, + projectionNext: null, + child: null, + parent: tParent, + projection: null, + styles: null, + stylesWithoutHost: null, + residualStyles: undefined, + classes: null, + classesWithoutHost: null, + residualClasses: undefined, + classBindings: 0 as any, + styleBindings: 0 as any, + }; + if (ngDevMode) { + // For performance reasons it is important that the tNode retains the same shape during runtime. + // (To make sure that all of the code is monomorphic.) For this reason we seal the object to + // prevent class transitions. + Object.seal(tNode); + } + return tNode; } @@ -2040,7 +2048,7 @@ export function setInputsForProperty( const index = inputs[i++] as number; const privateName = inputs[i++] as string; const instance = lView[index]; - ngDevMode && assertDataInRange(lView, index); + ngDevMode && assertIndexInRange(lView, index); const def = tView.data[index] as DirectiveDef; if (def.setInput !== null) { def.setInput!(instance, value, publicName, privateName); @@ -2055,7 +2063,7 @@ export function setInputsForProperty( */ export function textBindingInternal(lView: LView, index: number, value: string): void { ngDevMode && assertNotSame(value, NO_CHANGE as any, 'value should not be NO_CHANGE'); - ngDevMode && assertDataInRange(lView, index + HEADER_OFFSET); + ngDevMode && assertIndexInRange(lView, index + HEADER_OFFSET); const element = getNativeByIndex(index, lView) as any as RText; ngDevMode && assertDefined(element, 'native element should exist'); ngDevMode && ngDevMode.rendererSetText++; diff --git a/packages/core/src/render3/instructions/text.ts b/packages/core/src/render3/instructions/text.ts index a98d9df165..88a27e8656 100644 --- a/packages/core/src/render3/instructions/text.ts +++ b/packages/core/src/render3/instructions/text.ts @@ -5,11 +5,12 @@ * 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 {assertDataInRange, assertEqual} from '../../util/assert'; +import {assertEqual, assertIndexInRange} from '../../util/assert'; import {TElementNode, TNodeType} from '../interfaces/node'; import {HEADER_OFFSET, RENDERER, T_HOST} from '../interfaces/view'; import {appendChild, createTextNode} from '../node_manipulation'; import {getBindingIndex, getLView, getTView, setPreviousOrParentTNode} from '../state'; + import {getOrCreateTNode} from './shared'; @@ -31,7 +32,7 @@ export function ɵɵtext(index: number, value: string = ''): void { assertEqual( getBindingIndex(), tView.bindingStartIndex, 'text nodes should be created before any bindings'); - ngDevMode && assertDataInRange(lView, adjustedIndex); + ngDevMode && assertIndexInRange(lView, adjustedIndex); const tNode = tView.firstCreatePass ? getOrCreateTNode(tView, lView[T_HOST], index, TNodeType.Element, null, null) : diff --git a/packages/core/src/render3/interfaces/i18n.ts b/packages/core/src/render3/interfaces/i18n.ts index f379ed9125..7b85b495f5 100644 --- a/packages/core/src/render3/interfaces/i18n.ts +++ b/packages/core/src/render3/interfaces/i18n.ts @@ -346,6 +346,14 @@ export interface TIcu { */ vars: number[]; + /** + * Currently selected ICU case pointer. + * + * `lView[currentCaseLViewIndex]` stores the currently selected case. This is needed to know how + * to clean up the current case when transitioning no the new case. + */ + currentCaseLViewIndex: number; + /** * An optional array of child/sub ICUs. * diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index 4684232065..462105d2f4 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -709,13 +709,6 @@ export interface TIcuContainerNode extends TNode { parent: TElementNode|TElementContainerNode|null; tViews: null; projection: null; - /** - * Indicates the current active case for an ICU expression. - * It is null when there is no active case. - * - */ - // FIXME(misko): This is at a wrong location as activeCase is `LView` (not `TView`) concern - activeCaseIndex: number|null; } /** Static data for a view */ diff --git a/packages/core/src/render3/pure_function.ts b/packages/core/src/render3/pure_function.ts index 2abb42d10c..6db19df34d 100644 --- a/packages/core/src/render3/pure_function.ts +++ b/packages/core/src/render3/pure_function.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {assertDataInRange} from '../util/assert'; +import {assertIndexInRange} from '../util/assert'; import {bindingUpdated, bindingUpdated2, bindingUpdated3, bindingUpdated4, getBinding, updateBinding} from './bindings'; import {LView} from './interfaces/view'; import {getBindingRoot, getLView} from './state'; @@ -287,7 +287,7 @@ export function ɵɵpureFunctionV( * it to `undefined`. */ function getPureFunctionReturnValue(lView: LView, returnValueIndex: number) { - ngDevMode && assertDataInRange(lView, returnValueIndex); + ngDevMode && assertIndexInRange(lView, returnValueIndex); const lastReturnValue = lView[returnValueIndex]; return lastReturnValue === NO_CHANGE ? undefined : lastReturnValue; } diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index 63eaf2bd7d..58e587703e 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -15,7 +15,7 @@ import {ElementRef as ViewEngine_ElementRef} from '../linker/element_ref'; import {QueryList} from '../linker/query_list'; import {TemplateRef as ViewEngine_TemplateRef} from '../linker/template_ref'; import {ViewContainerRef} from '../linker/view_container_ref'; -import {assertDataInRange, assertDefined, throwError} from '../util/assert'; +import {assertDefined, assertIndexInRange, throwError} from '../util/assert'; import {stringify} from '../util/stringify'; import {assertFirstCreatePass, assertLContainer} from './assert'; @@ -140,7 +140,7 @@ class TQueries_ implements TQueries { } getByIndex(index: number): TQuery { - ngDevMode && assertDataInRange(this.queries, index); + ngDevMode && assertIndexInRange(this.queries, index); return this.queries[index]; } @@ -358,7 +358,7 @@ function materializeViewResults( // null as a placeholder result.push(null); } else { - ngDevMode && assertDataInRange(tViewData, matchedNodeIdx); + ngDevMode && assertIndexInRange(tViewData, matchedNodeIdx); const tNode = tViewData[matchedNodeIdx] as TNode; result.push(createResultForNode(lView, tNode, tQueryMatches[i + 1], tQuery.metadata.read)); } @@ -551,7 +551,7 @@ export function ɵɵloadQuery(): QueryList { function loadQueryInternal(lView: LView, queryIndex: number): QueryList { ngDevMode && assertDefined(lView[QUERIES], 'LQueries should be defined when trying to load a query'); - ngDevMode && assertDataInRange(lView[QUERIES]!.queries, queryIndex); + ngDevMode && assertIndexInRange(lView[QUERIES]!.queries, queryIndex); return lView[QUERIES]!.queries[queryIndex].queryList; } diff --git a/packages/core/src/render3/styling/style_binding_list.ts b/packages/core/src/render3/styling/style_binding_list.ts index 4fde39fcd4..d05bde5f93 100644 --- a/packages/core/src/render3/styling/style_binding_list.ts +++ b/packages/core/src/render3/styling/style_binding_list.ts @@ -7,7 +7,7 @@ */ import {KeyValueArray, keyValueArrayIndexOf} from '../../util/array_utils'; -import {assertDataInRange, assertEqual, assertNotEqual} from '../../util/assert'; +import {assertEqual, assertIndexInRange, assertNotEqual} from '../../util/assert'; import {assertFirstUpdatePass} from '../assert'; import {TNode} from '../interfaces/node'; import {getTStylingRangeNext, getTStylingRangePrev, setTStylingRangeNext, setTStylingRangeNextDuplicate, setTStylingRangePrev, setTStylingRangePrevDuplicate, toTStylingRange, TStylingKey, TStylingKeyPrimitive, TStylingRange} from '../interfaces/styling'; @@ -370,7 +370,7 @@ function markDuplicates( // - we are a map in which case we have to continue searching even after we find what we were // looking for since we are a wild card and everything needs to be flipped to duplicate. while (cursor !== 0 && (foundDuplicate === false || isMap)) { - ngDevMode && assertDataInRange(tData, cursor); + ngDevMode && assertIndexInRange(tData, cursor); const tStylingValueAtCursor = tData[cursor] as TStylingKey; const tStyleRangeAtCursor = tData[cursor + 1] as TStylingRange; if (isStylingMatch(tStylingValueAtCursor, tStylingKey)) { diff --git a/packages/core/src/render3/util/view_utils.ts b/packages/core/src/render3/util/view_utils.ts index 21bee1c5d6..4876a8a7d1 100644 --- a/packages/core/src/render3/util/view_utils.ts +++ b/packages/core/src/render3/util/view_utils.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {assertDataInRange, assertDefined, assertDomNode, assertGreaterThan, assertLessThan} from '../../util/assert'; +import {assertDefined, assertDomNode, assertGreaterThan, assertIndexInRange, assertLessThan} from '../../util/assert'; import {assertTNodeForLView} from '../assert'; import {LContainer, TYPE} from '../interfaces/container'; import {LContext, MONKEY_PATCH_KEY_NAME} from '../interfaces/context'; @@ -91,7 +91,7 @@ export function getNativeByIndex(index: number, lView: LView): RNode { */ export function getNativeByTNode(tNode: TNode, lView: LView): RNode { ngDevMode && assertTNodeForLView(tNode, lView); - ngDevMode && assertDataInRange(lView, tNode.index); + ngDevMode && assertIndexInRange(lView, tNode.index); const node: RNode = unwrapRNode(lView[tNode.index]); ngDevMode && !isProceduralRenderer(lView[RENDERER]) && assertDomNode(node); return node; @@ -125,13 +125,13 @@ export function getTNode(tView: TView, index: number): TNode { /** Retrieves a value from any `LView` or `TData`. */ export function load(view: LView|TData, index: number): T { - ngDevMode && assertDataInRange(view, index + HEADER_OFFSET); + ngDevMode && assertIndexInRange(view, index + HEADER_OFFSET); return view[index + HEADER_OFFSET]; } export function getComponentLViewByIndex(nodeIndex: number, hostView: LView): LView { // Could be an LView or an LContainer. If LContainer, unwrap to find LView. - ngDevMode && assertDataInRange(hostView, nodeIndex); + ngDevMode && assertIndexInRange(hostView, nodeIndex); const slotValue = hostView[nodeIndex]; const lView = isLView(slotValue) ? slotValue : slotValue[HOST]; return lView; diff --git a/packages/core/src/util/assert.ts b/packages/core/src/util/assert.ts index 1678b3bc53..1627149d56 100644 --- a/packages/core/src/util/assert.ts +++ b/packages/core/src/util/assert.ts @@ -110,7 +110,7 @@ export function assertDomNode(node: any): asserts node is Node { } -export function assertDataInRange(arr: any[], index: number) { +export function assertIndexInRange(arr: any[], index: number) { const maxLen = arr ? arr.length : 0; assertLessThan(index, maxLen, `Index expected to be less than ${maxLen} but got ${index}`); } diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index cbca1e1246..ec106a8381 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -14,7 +14,11 @@ import localeEs from '@angular/common/locales/es'; import localeRo from '@angular/common/locales/ro'; import {computeMsgId} from '@angular/compiler'; import {Component, ContentChild, ContentChildren, Directive, ElementRef, HostBinding, Input, LOCALE_ID, NO_ERRORS_SCHEMA, Pipe, PipeTransform, QueryList, RendererFactory2, TemplateRef, Type, ViewChild, ViewContainerRef, ɵsetDocument} from '@angular/core'; +import {getComponentDef} from '@angular/core/src/render3/definition'; import {setDelayProjection} from '@angular/core/src/render3/instructions/projection'; +import {TI18n, TIcu} from '@angular/core/src/render3/interfaces/i18n'; +import {DebugNode, HEADER_OFFSET, TVIEW} from '@angular/core/src/render3/interfaces/view'; +import {getComponentLView, loadLContext} from '@angular/core/src/render3/util/discovery_utils'; import {TestBed} from '@angular/core/testing'; import {clearTranslations, loadTranslations} from '@angular/localize'; import {By, ɵDomRendererFactory2 as DomRendererFactory2} from '@angular/platform-browser'; @@ -599,6 +603,217 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { }); }); + describe('dynamic TNodes', () => { + // When translation occurs the i18n system needs to create dynamic TNodes for the text + // nodes so that they can be correctly processed by the `addRemoveViewFromContainer`. + + function toTypeContent(n: DebugNode): string { + return `${n.type}(${n.html})`; + } + + it('should not create dynamic TNode when no i18n', () => { + const fixture = initWithTemplate(AppComp, `Hello World!`); + const lView = getComponentLView(fixture.componentInstance); + const hello_ = (fixture.nativeElement as Element).firstChild!; + const b = hello_.nextSibling!; + const world = b.firstChild!; + const exclamation = b.nextSibling!; + const lViewDebug = lView.debug!; + expect(lViewDebug.nodes.map(toTypeContent)).toEqual([ + 'Element(Hello )', 'Element()', 'Element(!)' + ]); + expect(lViewDebug.decls).toEqual({ + start: HEADER_OFFSET, + end: HEADER_OFFSET + 4, + length: 4, + content: [ + jasmine.objectContaining({index: HEADER_OFFSET + 0, l: hello_}), + jasmine.objectContaining({index: HEADER_OFFSET + 1, l: b}), + jasmine.objectContaining({index: HEADER_OFFSET + 2, l: world}), + jasmine.objectContaining({index: HEADER_OFFSET + 3, l: exclamation}), + ] + }); + expect(lViewDebug.i18n) + .toEqual( + {start: lViewDebug.vars.end, end: lViewDebug.expando.start, length: 0, content: []}); + }); + + it('should create dynamic TNode for text nodes', () => { + const fixture = + initWithTemplate(AppComp, `Hello World!`); + const lView = getComponentLView(fixture.componentInstance); + const hello_ = (fixture.nativeElement as Element).firstChild!; + const b = hello_.nextSibling!; + const world = b.firstChild!; + const exclamation = b.nextSibling!; + const container = exclamation.nextSibling!; + const lViewDebug = lView.debug!; + expect(lViewDebug.nodes.map(toTypeContent)).toEqual([ + 'ElementContainer()' + ]); + // This assertion shows that the translated nodes are correctly linked into the TNode tree. + expect(lViewDebug.nodes[0].children.map(toTypeContent)).toEqual([ + 'Element(Hello )', 'Element()', 'Element(!)' + ]); + // This assertion shows that the translated text is not part of decls + expect(lViewDebug.decls).toEqual({ + start: HEADER_OFFSET, + end: HEADER_OFFSET + 3, + length: 3, + content: [ + jasmine.objectContaining({index: HEADER_OFFSET + 0, l: container}), + jasmine.objectContaining({index: HEADER_OFFSET + 1}), + jasmine.objectContaining({index: HEADER_OFFSET + 2, l: b}), + ] + }); + // This assertion shows that the translated DOM elements (and corresponding TNode's are stored + // in i18n section of LView) + expect(lViewDebug.i18n).toEqual({ + start: lViewDebug.vars.end, + end: lViewDebug.expando.start, + length: 3, + content: [ + jasmine.objectContaining({index: HEADER_OFFSET + 3, l: hello_}), + jasmine.objectContaining({index: HEADER_OFFSET + 4, l: world}), + jasmine.objectContaining({index: HEADER_OFFSET + 5, l: exclamation}), + ] + }); + // This assertion shows the DOM operations which the i18n subsystem performed to update the + // DOM with translated text. The offsets in the debug text should match the offsets in the + // above assertions. + expect((lView[TVIEW]!.data[HEADER_OFFSET + 1]! as TI18n).create.debug).toEqual([ + 'lView[3] = document.createTextNode("Hello ")', + '(lView[0] as Element).appendChild(lView[3])', + '(lView[0] as Element).appendChild(lView[2])', + 'lView[4] = document.createTextNode("World")', + '(lView[2] as Element).appendChild(lView[4])', + 'setPreviousOrParentTNode(tView.data[2] as TNode)', + 'lView[5] = document.createTextNode("!")', + '(lView[0] as Element).appendChild(lView[5])', + ]); + }); + + describe('ICU', () => { + // In the case of ICUs we can't create TNodes for each ICU part, as different ICU instances + // may have different selections active and hence have different shape. In such a case + // a single `TIcuContainerNode` should be generated only. + it('should create a single dynamic TNode for ICU', () => { + const fixture = initWithTemplate(AppComp, ` + {count, plural, + =0 {just now} + =1 {one minute ago} + other {{{count}} minutes ago} + } + `); + const lView = getComponentLView(fixture.componentInstance); + const lViewDebug = lView.debug!; + expect((fixture.nativeElement as Element).textContent).toEqual('just now'); + const text_just_now = (fixture.nativeElement as Element).firstChild!; + const icuComment = text_just_now.nextSibling!; + expect(lViewDebug.nodes.map(toTypeContent)).toEqual(['IcuContainer()']); + // We want to ensure that the ICU container does not have any content! + // This is because the content is instance dependent and therefore can't be shared + // across `TNode`s. + expect(lViewDebug.nodes[0].children.map(toTypeContent)).toEqual([ + 'Element(just now)', // FIXME(misko): This should not be here. The content of the ICU is + // instance specific and as such can't be encoded in the tNodes. + ]); + expect(lViewDebug.decls).toEqual({ + start: HEADER_OFFSET, + end: HEADER_OFFSET + 1, + length: 1, + content: [ + jasmine.objectContaining({ + t: jasmine.objectContaining({ + vars: 3, // one slot for: the `` + // one slot for: the last selected ICU case. + // one slot for: the actual text node to attach. + create: jasmine.any(Object), + update: jasmine.any(Object), + icus: [jasmine.any(Object)], + }), + l: null + }), + ] + }); + expect(((lViewDebug.decls.content[0].t as TI18n).create.debug)).toEqual([ + 'lView[3] = document.createComment("ICU 3")', + '(lView[0] as Element).appendChild(lView[3])', + ]); + expect(((lViewDebug.decls.content[0].t as TI18n).update.debug)).toEqual([ + 'if (mask & 0b1) { icuSwitchCase(lView[3] as Comment, 0, `${lView[1]}`); }', + 'if (mask & 0b11) { icuUpdateCase(lView[3] as Comment, 0); }', + ]); + const tIcu = (lViewDebug.decls.content[0].t as TI18n).icus![0]; + expect(tIcu.cases).toEqual(['0', '1', 'other']); + // Case: '0' + expect(tIcu.create[0].debug).toEqual([ + 'lView[5] = document.createTextNode("just now")', + '(lView[3] as Element).appendChild(lView[5])', + ]); + expect(tIcu.remove[0].debug).toEqual(['(lView[0] as Element).remove(lView[5])']); + expect(tIcu.update[0].debug).toEqual([]); + + // Case: '1' + expect(tIcu.create[1].debug).toEqual([ + 'lView[5] = document.createTextNode("one minute ago")', + '(lView[3] as Element).appendChild(lView[5])', + ]); + expect(tIcu.remove[1].debug).toEqual(['(lView[0] as Element).remove(lView[5])']); + expect(tIcu.update[1].debug).toEqual([]); + + // Case: 'other' + expect(tIcu.create[2].debug).toEqual([ + 'lView[5] = document.createTextNode("")', + '(lView[3] as Element).appendChild(lView[5])', + ]); + expect(tIcu.remove[2].debug).toEqual(['(lView[0] as Element).remove(lView[5])']); + expect(tIcu.update[2].debug).toEqual([ + 'if (mask & 0b10) { (lView[5] as Text).textContent = `${lView[2]} minutes ago`; }' + ]); + + expect(lViewDebug.i18n).toEqual({ + start: lViewDebug.vars.end, + end: lViewDebug.expando.start, + length: 3, + content: [ + // ICU anchor ``. + jasmine.objectContaining({index: HEADER_OFFSET + 3, l: icuComment}), + // ICU `TIcu.currentCaseLViewIndex` storage location + jasmine.objectContaining({ + index: HEADER_OFFSET + 4, + t: null, + l: 0, // The current ICU case + }), + jasmine.objectContaining({index: HEADER_OFFSET + 5, l: text_just_now}), + ] + }); + }); + + // FIXME(misko): re-enable and fix this use case. + xit('should support multiple ICUs', () => { + const fixture = initWithTemplate(AppComp, ` + {count, plural, + =0 {just now} + =1 {one minute ago} + other {{{count}} minutes ago} + } + {count, plural, + =0 {just now} + =1 {one minute ago} + other {{{count}} minutes ago} + } + `); + const lView = getComponentLView(fixture.componentInstance); + expect(lView.debug!.nodes.map(toTypeContent)).toEqual(['IcuContainer()']); + // We want to ensure that the ICU container does not have any content! + // This is because the content is instance dependent and therefore can't be shared + // across `TNode`s. + expect(lView.debug!.nodes[0].children.map(toTypeContent)).toEqual([]); + }); + }); + }); + describe('should support ICU expressions', () => { it('with no root node', () => { loadTranslations({ @@ -690,19 +905,19 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { other {({{name}})} }`); expect(fixture.nativeElement.innerHTML) - .toEqual(`
aucun email! - (Angular)
`); + .toEqual(`
aucun email! - (Angular)
`); fixture.componentRef.instance.count = 4; fixture.detectChanges(); expect(fixture.nativeElement.innerHTML) .toEqual( - `
4 emails - (Angular)
`); + `
4 emails - (Angular)
`); fixture.componentRef.instance.count = 0; fixture.componentRef.instance.name = 'John'; fixture.detectChanges(); expect(fixture.nativeElement.innerHTML) - .toEqual(`
aucun email! - (John)
`); + .toEqual(`
aucun email! - (John)
`); }); it('with custom interpolation config', () => { @@ -740,20 +955,20 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { }`); expect(fixture.nativeElement.innerHTML) .toEqual( - `
aucun email! - (Angular)
`); + `
aucun email! - (Angular)
`); fixture.componentRef.instance.count = 4; fixture.detectChanges(); expect(fixture.nativeElement.innerHTML) .toEqual( - `
4 emails - (Angular)
`); + `
4 emails - (Angular)
`); fixture.componentRef.instance.count = 0; fixture.componentRef.instance.name = 'John'; fixture.detectChanges(); expect(fixture.nativeElement.innerHTML) .toEqual( - `
aucun email! - (John)
`); + `
aucun email! - (John)
`); }); it('inside template directives', () => { @@ -1435,6 +1650,54 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { // checking the second ICU case expect(fixture.nativeElement.textContent.trim()).toBe('deux articles'); }); + + // FIXME(misko): re-enable and fix this use case. Root cause is that + // `addRemoveViewFromContainer` needs to understand ICU + xit('should handle select expressions without an `other` parameter inside a template', () => { + const fixture = initWithTemplate(AppComp, ` + {item.value, select, 0 {A} 1 {B} 2 {C}} + `); + fixture.componentInstance.items = [{value: 0}, {value: 1}, {value: 1337}]; + fixture.detectChanges(); + const p = fixture.nativeElement.querySelector('p'); + const lContext = loadLContext(p); + const lView = lContext.lView; + const nodeIndex = lContext.nodeIndex; + const tView = lView[TVIEW]; + const i18n = tView.data[nodeIndex + 1] as unknown as TI18n; + expect(fixture.nativeElement.textContent.trim()).toBe('AB'); + + fixture.componentInstance.items[0].value = 2; + fixture.detectChanges(); + expect(fixture.nativeElement.textContent.trim()).toBe('CB'); + fail('testing'); + }); + + it('should render an element whose case did not match initially', () => { + const fixture = initWithTemplate(AppComp, ` +

{item.value, select, 0 {A} 1 {B} 2 {C}}

+ `); + fixture.componentInstance.items = [{value: 0}, {value: 1}, {value: 1337}]; + fixture.detectChanges(); + expect(fixture.nativeElement.textContent.trim()).toBe('AB'); + + fixture.componentInstance.items[2].value = 2; + fixture.detectChanges(); + expect(fixture.nativeElement.textContent.trim()).toBe('ABC'); + }); + + it('should remove an element whose case matched initially, but does not anymore', () => { + const fixture = initWithTemplate(AppComp, ` +

{item.value, select, 0 {A} 1 {B} 2 {C}}

+ `); + fixture.componentInstance.items = [{value: 0}, {value: 1}]; + fixture.detectChanges(); + expect(fixture.nativeElement.textContent.trim()).toBe('AB'); + + fixture.componentInstance.items[0].value = 1337; + fixture.detectChanges(); + expect(fixture.nativeElement.textContent.trim()).toBe('B'); + }); }); describe('should support attributes', () => { diff --git a/packages/core/test/render3/i18n_spec.ts b/packages/core/test/render3/i18n_spec.ts index 6d46484845..20f2df3272 100644 --- a/packages/core/test/render3/i18n_spec.ts +++ b/packages/core/test/render3/i18n_spec.ts @@ -261,7 +261,7 @@ describe('Runtime i18n', () => { }, null, nbConsts, index) as TI18n; expect(opCodes).toEqual({ - vars: 5, + vars: 6, update: debugMatch([ 'if (mask & 0b1) { icuSwitchCase(lView[1] as Comment, 0, `${lView[1]}`); }', 'if (mask & 0b11) { icuUpdateCase(lView[1] as Comment, 0); }', @@ -272,60 +272,61 @@ describe('Runtime i18n', () => { ]), icus: [{ type: 1, - vars: [4, 3, 3], + currentCaseLViewIndex: 22, + vars: [5, 4, 4], childIcus: [[], [], []], cases: ['0', '1', 'other'], create: [ debugMatch([ - 'lView[2] = document.createTextNode("no ")', - '(lView[1] as Element).appendChild(lView[2])', - 'lView[3] = document.createElement("b")', + 'lView[3] = document.createTextNode("no ")', '(lView[1] as Element).appendChild(lView[3])', - '(lView[3] as Element).setAttribute("title", "none")', - 'lView[4] = document.createTextNode("emails")', - '(lView[3] as Element).appendChild(lView[4])', - 'lView[5] = document.createTextNode("!")', - '(lView[1] as Element).appendChild(lView[5])' + 'lView[4] = document.createElement("b")', + '(lView[1] as Element).appendChild(lView[4])', + '(lView[4] as Element).setAttribute("title", "none")', + 'lView[5] = document.createTextNode("emails")', + '(lView[4] as Element).appendChild(lView[5])', + 'lView[6] = document.createTextNode("!")', + '(lView[1] as Element).appendChild(lView[6])', ]), debugMatch([ - 'lView[2] = document.createTextNode("one ")', - '(lView[1] as Element).appendChild(lView[2])', - 'lView[3] = document.createElement("i")', + 'lView[3] = document.createTextNode("one ")', '(lView[1] as Element).appendChild(lView[3])', - 'lView[4] = document.createTextNode("email")', - '(lView[3] as Element).appendChild(lView[4])' + 'lView[4] = document.createElement("i")', + '(lView[1] as Element).appendChild(lView[4])', + 'lView[5] = document.createTextNode("email")', + '(lView[4] as Element).appendChild(lView[5])', ]), debugMatch([ - 'lView[2] = document.createTextNode("")', - '(lView[1] as Element).appendChild(lView[2])', - 'lView[3] = document.createElement("span")', + 'lView[3] = document.createTextNode("")', '(lView[1] as Element).appendChild(lView[3])', - 'lView[4] = document.createTextNode("emails")', - '(lView[3] as Element).appendChild(lView[4])' + 'lView[4] = document.createElement("span")', + '(lView[1] as Element).appendChild(lView[4])', + 'lView[5] = document.createTextNode("emails")', + '(lView[4] as Element).appendChild(lView[5])', ]) ], remove: [ debugMatch([ - '(lView[0] as Element).remove(lView[2])', - '(lView[0] as Element).remove(lView[4])', '(lView[0] as Element).remove(lView[3])', '(lView[0] as Element).remove(lView[5])', + '(lView[0] as Element).remove(lView[4])', + '(lView[0] as Element).remove(lView[6])', ]), debugMatch([ - '(lView[0] as Element).remove(lView[2])', - '(lView[0] as Element).remove(lView[4])', '(lView[0] as Element).remove(lView[3])', + '(lView[0] as Element).remove(lView[5])', + '(lView[0] as Element).remove(lView[4])', ]), debugMatch([ - '(lView[0] as Element).remove(lView[2])', - '(lView[0] as Element).remove(lView[4])', '(lView[0] as Element).remove(lView[3])', + '(lView[0] as Element).remove(lView[5])', + '(lView[0] as Element).remove(lView[4])', ]) ], update: [ debugMatch([]), debugMatch([]), debugMatch([ - 'if (mask & 0b1) { (lView[2] as Text).textContent = `${lView[1]} `; }', - 'if (mask & 0b10) { (lView[3] as Element).setAttribute(\'title\', `${lView[2]}`); }' + 'if (mask & 0b1) { (lView[3] as Text).textContent = `${lView[1]} `; }', + 'if (mask & 0b10) { (lView[4] as Element).setAttribute(\'title\', `${lView[2]}`); }' ]) ] }] @@ -355,7 +356,7 @@ describe('Runtime i18n', () => { const nestedTIcuIndex = 0; expect(opCodes).toEqual({ - vars: 6, + vars: 9, create: debugMatch([ 'lView[1] = document.createComment("ICU 1")', '(lView[0] as Element).appendChild(lView[1])' @@ -367,27 +368,28 @@ describe('Runtime i18n', () => { icus: [ { type: 0, - vars: [1, 1, 1], + vars: [2, 2, 2], + currentCaseLViewIndex: 26, childIcus: [[], [], []], cases: ['cat', 'dog', 'other'], create: [ debugMatch([ - 'lView[5] = document.createTextNode("cats")', - '(lView[3] as Element).appendChild(lView[5])' + 'lView[7] = document.createTextNode("cats")', + '(lView[4] as Element).appendChild(lView[7])' ]), debugMatch([ - 'lView[5] = document.createTextNode("dogs")', - '(lView[3] as Element).appendChild(lView[5])' + 'lView[7] = document.createTextNode("dogs")', + '(lView[4] as Element).appendChild(lView[7])' ]), debugMatch([ - 'lView[5] = document.createTextNode("animals")', - '(lView[3] as Element).appendChild(lView[5])' + 'lView[7] = document.createTextNode("animals")', + '(lView[4] as Element).appendChild(lView[7])' ]), ], remove: [ - debugMatch(['(lView[0] as Element).remove(lView[5])']), - debugMatch(['(lView[0] as Element).remove(lView[5])']), - debugMatch(['(lView[0] as Element).remove(lView[5])']) + debugMatch(['(lView[0] as Element).remove(lView[7])']), + debugMatch(['(lView[0] as Element).remove(lView[7])']), + debugMatch(['(lView[0] as Element).remove(lView[7])']) ], update: [ debugMatch([]), @@ -397,36 +399,37 @@ describe('Runtime i18n', () => { }, { type: 1, - vars: [1, 4], + vars: [2, 6], childIcus: [[], [0]], + currentCaseLViewIndex: 22, cases: ['0', 'other'], create: [ debugMatch([ - 'lView[2] = document.createTextNode("zero")', - '(lView[1] as Element).appendChild(lView[2])' + 'lView[3] = document.createTextNode("zero")', + '(lView[1] as Element).appendChild(lView[3])' ]), debugMatch([ - 'lView[2] = document.createTextNode("")', - '(lView[1] as Element).appendChild(lView[2])', - 'lView[3] = document.createComment("nested ICU 0")', + 'lView[3] = document.createTextNode("")', '(lView[1] as Element).appendChild(lView[3])', - 'lView[4] = document.createTextNode("!")', - '(lView[1] as Element).appendChild(lView[4])' + 'lView[4] = document.createComment("nested ICU 0")', + '(lView[1] as Element).appendChild(lView[4])', + 'lView[5] = document.createTextNode("!")', + '(lView[1] as Element).appendChild(lView[5])' ]), ], remove: [ - debugMatch(['(lView[0] as Element).remove(lView[2])']), + debugMatch(['(lView[0] as Element).remove(lView[3])']), debugMatch([ - '(lView[0] as Element).remove(lView[2])', '(lView[0] as Element).remove(lView[4])', - 'removeNestedICU(0)', '(lView[0] as Element).remove(lView[3])' + '(lView[0] as Element).remove(lView[3])', '(lView[0] as Element).remove(lView[5])', + 'removeNestedICU(0)', '(lView[0] as Element).remove(lView[4])' ]), ], update: [ debugMatch([]), debugMatch([ - 'if (mask & 0b1) { (lView[2] as Text).textContent = `${lView[1]} `; }', - 'if (mask & 0b10) { icuSwitchCase(lView[3] as Comment, 0, `${lView[2]}`); }', - 'if (mask & 0b10) { icuUpdateCase(lView[3] as Comment, 0); }' + 'if (mask & 0b1) { (lView[3] as Text).textContent = `${lView[1]} `; }', + 'if (mask & 0b10) { icuSwitchCase(lView[4] as Comment, 0, `${lView[2]}`); }', + 'if (mask & 0b10) { icuUpdateCase(lView[4] as Comment, 0); }' ]), ] }