From bc5005e35bf1c8fe17f6a7ea4961cc09f6964018 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Tue, 13 Oct 2020 16:24:58 -0700 Subject: [PATCH] refactor(core): cleanup i18n/icu data structures (#39233) - Made `*OpCodes` array branded for safer type checking. - Simplify `I18NRemoveOpCodes` encoding. - Broke out `IcuCreateOpCodes` from `I18nMutableOpCodes`. PR Close #39233 --- packages/core/src/render3/i18n/i18n_apply.ts | 36 ++- packages/core/src/render3/i18n/i18n_debug.ts | 63 +++-- packages/core/src/render3/i18n/i18n_parse.ts | 47 ++-- .../i18n_icu_container_visitor.ts | 19 +- packages/core/src/render3/interfaces/i18n.ts | 221 +++++++++--------- packages/core/src/render3/util/debug_utils.ts | 2 +- .../core/test/render3/i18n/i18n_parse_spec.ts | 20 +- packages/core/test/render3/i18n/i18n_spec.ts | 36 +-- packages/core/test/render3/i18n_debug_spec.ts | 76 +++--- packages/core/test/render3/matchers.ts | 4 +- 10 files changed, 275 insertions(+), 249 deletions(-) diff --git a/packages/core/src/render3/i18n/i18n_apply.ts b/packages/core/src/render3/i18n/i18n_apply.ts index 499a399da5..7b7148b998 100644 --- a/packages/core/src/render3/i18n/i18n_apply.ts +++ b/packages/core/src/render3/i18n/i18n_apply.ts @@ -11,7 +11,7 @@ import {assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertInde import {assertIndexInExpandoRange, assertTIcu} from '../assert'; import {attachPatchData} from '../context_discovery'; import {elementPropertyInternal, setElementAttribute} from '../instructions/shared'; -import {ELEMENT_MARKER, getCurrentICUCaseIndex, getParentFromI18nMutateOpCode, getRefFromI18nMutateOpCode, I18nCreateOpCode, I18nCreateOpCodes, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, IcuType, TI18n, TIcu} from '../interfaces/i18n'; +import {ELEMENT_MARKER, getCurrentICUCaseIndex, getParentFromIcuCreateOpCode, getRefFromIcuCreateOpCode, I18nCreateOpCode, I18nCreateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, IcuCreateOpCode, IcuCreateOpCodes, IcuType, TI18n, TIcu} from '../interfaces/i18n'; import {TNode} from '../interfaces/node'; import {RElement, RNode, RText} from '../interfaces/renderer'; import {SanitizerFn} from '../interfaces/sanitization'; @@ -122,7 +122,7 @@ export function applyCreateOpCodes( * @param anchorRNode place where the i18n node should be inserted. */ export function applyMutableOpCodes( - tView: TView, mutableOpCodes: I18nMutateOpCodes, lView: LView, anchorRNode: RNode): void { + tView: TView, mutableOpCodes: IcuCreateOpCodes, lView: LView, anchorRNode: RNode): void { ngDevMode && assertDomNode(anchorRNode); const renderer = lView[RENDERER]; // `rootIdx` represents the node into which all inserts happen. @@ -143,9 +143,9 @@ export function applyMutableOpCodes( lView[textNodeIndex] = createTextNode(renderer, opCode); } } else if (typeof opCode == 'number') { - switch (opCode & I18nMutateOpCode.MASK_INSTRUCTION) { - case I18nMutateOpCode.AppendChild: - const parentIdx = getParentFromI18nMutateOpCode(opCode); + switch (opCode & IcuCreateOpCode.MASK_INSTRUCTION) { + case IcuCreateOpCode.AppendChild: + const parentIdx = getParentFromIcuCreateOpCode(opCode); if (rootIdx === null) { // The first operation should save the `rootIdx` because the first operation // must insert into the root. (Only subsequent operations can insert into a dynamic @@ -169,7 +169,7 @@ export function applyMutableOpCodes( // create the elements. When the `LView` gets later added to a parent these "root" nodes // get picked up and added. ngDevMode && assertDomNode(parentRNode); - const refIdx = getRefFromI18nMutateOpCode(opCode); + const refIdx = getRefFromIcuCreateOpCode(opCode); ngDevMode && assertGreaterThan(refIdx, HEADER_OFFSET, 'Missing ref'); // `unwrapRNode` is not needed here as all of these point to RNodes as part of the i18n // which can't have components. @@ -188,8 +188,8 @@ export function applyMutableOpCodes( } } break; - case I18nMutateOpCode.Attr: - const elementNodeIndex = opCode >>> I18nMutateOpCode.SHIFT_REF; + case IcuCreateOpCode.Attr: + const elementNodeIndex = opCode >>> IcuCreateOpCode.SHIFT_REF; const attrName = mutableOpCodes[++i] as string; const attrValue = mutableOpCodes[++i] as string; // This code is used for ICU expressions only, since we don't support @@ -393,17 +393,15 @@ function applyIcuSwitchCaseRemove(tView: TView, tIcu: TIcu, lView: LView) { let activeCaseIndex = getCurrentICUCaseIndex(tIcu, lView); 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: - nativeRemoveNode( - lView[RENDERER], getNativeByIndex(nodeOrIcuIndex - HEADER_OFFSET, lView)); - break; - case I18nMutateOpCode.RemoveNestedIcu: - applyIcuSwitchCaseRemove(tView, getTIcu(tView, nodeOrIcuIndex)!, lView); - break; + for (let i = 0; i < removeCodes.length; i++) { + const nodeOrIcuIndex = removeCodes[i] as number; + if (nodeOrIcuIndex > 0) { + // Positive numbers are `RNode`s. + const rNode = getNativeByIndex(nodeOrIcuIndex, lView); + rNode !== null && nativeRemoveNode(lView[RENDERER], rNode); + } else { + // Negative numbers are ICUs + applyIcuSwitchCaseRemove(tView, getTIcu(tView, ~nodeOrIcuIndex)!, lView); } } } diff --git a/packages/core/src/render3/i18n/i18n_debug.ts b/packages/core/src/render3/i18n/i18n_debug.ts index c048e9ee2b..edac425025 100644 --- a/packages/core/src/render3/i18n/i18n_debug.ts +++ b/packages/core/src/render3/i18n/i18n_debug.ts @@ -7,7 +7,7 @@ */ import {assertNumber, assertString} from '../../util/assert'; -import {ELEMENT_MARKER, getInstructionFromI18nMutateOpCode, getParentFromI18nMutateOpCode, getRefFromI18nMutateOpCode, I18nCreateOpCode, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER} from '../interfaces/i18n'; +import {ELEMENT_MARKER, getInstructionFromIcuCreateOpCode, getParentFromIcuCreateOpCode, getRefFromIcuCreateOpCode, I18nCreateOpCode, I18nCreateOpCodes, I18nRemoveOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, IcuCreateOpCode, IcuCreateOpCodes} from '../interfaces/i18n'; /** @@ -21,8 +21,8 @@ import {ELEMENT_MARKER, getInstructionFromI18nMutateOpCode, getParentFromI18nMut * @param opcodes `I18nCreateOpCodes` if invoked as a function. */ export function i18nCreateOpCodesToString( - this: I18nUpdateOpCodes|void, opcodes?: I18nUpdateOpCodes): string[] { - const createOpCodes: I18nUpdateOpCodes = opcodes || (Array.isArray(this) ? this : []); + this: I18nCreateOpCodes|void, opcodes?: I18nCreateOpCodes): string[] { + const createOpCodes: I18nCreateOpCodes = opcodes || (Array.isArray(this) ? this : [] as any); let lines: string[] = []; for (let i = 0; i < createOpCodes.length; i++) { const opCode = createOpCodes[i++] as any; @@ -103,35 +103,31 @@ export function i18nUpdateOpCodesToString( } /** - * Converts `I18nMutableOpCodes` array into a human readable format. + * Converts `I18nCreateOpCodes` array into a human readable format. * - * This function is attached to the `I18nMutableOpCodes.debug` if `ngDevMode` is enabled. This + * This function is attached to the `I18nCreateOpCodes.debug` if `ngDevMode` is enabled. This * function provides a human readable view of the opcodes. This is useful when debugging the * application as well as writing more readable tests. * - * @param this `I18nMutableOpCodes` if attached as a method. - * @param opcodes `I18nMutableOpCodes` if invoked as a function. + * @param this `I18nCreateOpCodes` if attached as a method. + * @param opcodes `I18nCreateOpCodes` if invoked as a function. */ -export function i18nMutateOpCodesToString( - this: I18nMutateOpCodes|void, opcodes?: I18nMutateOpCodes): string[] { +export function icuCreateOpCodesToString( + this: IcuCreateOpCodes|void, opcodes?: IcuCreateOpCodes): string[] { const parser = new OpCodeParser(opcodes || (Array.isArray(this) ? this : [])); let lines: string[] = []; function consumeOpCode(opCode: number): string { - const parent = getParentFromI18nMutateOpCode(opCode); - const ref = getRefFromI18nMutateOpCode(opCode); - switch (getInstructionFromI18nMutateOpCode(opCode)) { - case I18nMutateOpCode.AppendChild: + const parent = getParentFromIcuCreateOpCode(opCode); + const ref = getRefFromIcuCreateOpCode(opCode); + switch (getInstructionFromIcuCreateOpCode(opCode)) { + case IcuCreateOpCode.AppendChild: return `(lView[${parent}] as Element).appendChild(lView[${lastRef}])`; - case I18nMutateOpCode.Remove: - return `(lView[${parent}] as Element).remove(lView[${ref}])`; - case I18nMutateOpCode.Attr: + case IcuCreateOpCode.Attr: return `(lView[${ref}] as Element).setAttribute("${parser.consumeString()}", "${ parser.consumeString()}")`; - case I18nMutateOpCode.RemoveNestedIcu: - return `removeNestedICU(${ref})`; } - throw new Error('Unexpected OpCode'); + throw new Error('Unexpected OpCode: ' + getInstructionFromIcuCreateOpCode(opCode)); } let lastRef = -1; @@ -159,6 +155,35 @@ export function i18nMutateOpCodesToString( return lines; } +/** + * Converts `I18nRemoveOpCodes` array into a human readable format. + * + * This function is attached to the `I18nRemoveOpCodes.debug` if `ngDevMode` is enabled. This + * function provides a human readable view of the opcodes. This is useful when debugging the + * application as well as writing more readable tests. + * + * @param this `I18nRemoveOpCodes` if attached as a method. + * @param opcodes `I18nRemoveOpCodes` if invoked as a function. + */ +export function i18nRemoveOpCodesToString( + this: I18nRemoveOpCodes|void, opcodes?: I18nRemoveOpCodes): string[] { + const removeCodes = opcodes || (Array.isArray(this) ? this : []); + let lines: string[] = []; + + for (let i = 0; i < removeCodes.length; i++) { + const nodeOrIcuIndex = removeCodes[i] as number; + if (nodeOrIcuIndex > 0) { + // Positive numbers are `RNode`s. + lines.push(`remove(lView[${nodeOrIcuIndex}])`); + } else { + // Negative numbers are ICUs + lines.push(`removeNestedICU(${~nodeOrIcuIndex})`); + } + } + + return lines; +} + class OpCodeParser { i: number = 0; diff --git a/packages/core/src/render3/i18n/i18n_parse.ts b/packages/core/src/render3/i18n/i18n_parse.ts index 11be9980e9..ec39a9ecab 100644 --- a/packages/core/src/render3/i18n/i18n_parse.ts +++ b/packages/core/src/render3/i18n/i18n_parse.ts @@ -16,7 +16,7 @@ import {CharCode} from '../../util/char_code'; import {loadIcuContainerVisitor} from '../instructions/i18n_icu_container_visitor'; import {allocExpando, createTNodeAtIndex, elementAttributeInternal, setInputsForProperty, setNgReflectProperties} from '../instructions/shared'; import {getDocument} from '../interfaces/document'; -import {ELEMENT_MARKER, ensureIcuContainerVisitorLoaded, I18nCreateOpCode, I18nCreateOpCodes, I18nMutateOpCode, i18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, IcuExpression, IcuType, TI18n, TIcu} from '../interfaces/i18n'; +import {ELEMENT_MARKER, ensureIcuContainerVisitorLoaded, I18nCreateOpCode, I18nCreateOpCodes, I18nRemoveOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, icuCreateOpCode, IcuCreateOpCode, IcuCreateOpCodes, IcuExpression, IcuType, TI18n, TIcu} from '../interfaces/i18n'; import {TNode, TNodeType} from '../interfaces/node'; import {RComment, RElement} from '../interfaces/renderer'; import {SanitizerFn} from '../interfaces/sanitization'; @@ -25,7 +25,7 @@ import {getCurrentParentTNode, getCurrentTNode, setCurrentTNode} from '../state' import {attachDebugGetter} from '../util/debug_utils'; import {getNativeByIndex, getTNode} from '../util/view_utils'; -import {i18nCreateOpCodesToString, i18nMutateOpCodesToString, i18nUpdateOpCodesToString} from './i18n_debug'; +import {i18nCreateOpCodesToString, i18nRemoveOpCodesToString, i18nUpdateOpCodesToString, icuCreateOpCodesToString} from './i18n_debug'; import {addTNodeAndUpdateInsertBeforeIndex} from './i18n_insert_before_index'; import {createTNodePlaceholder, setTIcu, setTNodeInsertBeforeIndex} from './i18n_util'; @@ -70,8 +70,8 @@ export function i18nStartFirstCreatePass( tView: TView, parentTNodeIndex: number, lView: LView, index: number, message: string, subTemplateIndex: number) { const rootTNode = getCurrentParentTNode(); - const createOpCodes: I18nCreateOpCodes = []; - const updateOpCodes: I18nUpdateOpCodes = []; + const createOpCodes: I18nCreateOpCodes = [] as any; + const updateOpCodes: I18nUpdateOpCodes = [] as any; const existingTNodeStack: TNode[][] = [[]]; if (ngDevMode) { attachDebugGetter(createOpCodes, i18nCreateOpCodesToString); @@ -158,7 +158,7 @@ function createTNodeAndAddOpCode( let parentTNode = getCurrentParentTNode(); if (rootTNode === parentTNode) { - // FIXME(misko): A null `parentTNode` should represent when we fall of the `LView` boundry. + // FIXME(misko): A null `parentTNode` should represent when we fall of the `LView` boundary. // (there is no parent), but in some circumstances (because we are inconsistent about how we set // `previousOrParentTNode`) it could point to `rootTNode` So this is a work around. parentTNode = null; @@ -228,7 +228,7 @@ export function i18nAttributesFirstPass( lView: LView, tView: TView, index: number, values: string[]) { const previousElement = getCurrentTNode()!; const previousElementIndex = previousElement.index; - const updateOpCodes: I18nUpdateOpCodes = []; + const updateOpCodes: I18nUpdateOpCodes = [] as any; if (ngDevMode) { attachDebugGetter(updateOpCodes, i18nUpdateOpCodesToString); } @@ -583,12 +583,12 @@ export function i18nParseTextIntoPartsAndICU(pattern: string): (string|IcuExpres export function parseIcuCase( tView: TView, tIcu: TIcu, lView: LView, updateOpCodes: I18nUpdateOpCodes, parentIdx: number, caseName: string, unsafeCaseHtml: string, nestedIcus: IcuExpression[]): number { - const create: I18nMutateOpCodes = []; - const remove: I18nMutateOpCodes = []; - const update: I18nUpdateOpCodes = []; + const create: IcuCreateOpCodes = [] as any; + const remove: I18nRemoveOpCodes = [] as any; + const update: I18nUpdateOpCodes = [] as any; if (ngDevMode) { - attachDebugGetter(create, i18nMutateOpCodesToString); - attachDebugGetter(remove, i18nMutateOpCodesToString); + attachDebugGetter(create, icuCreateOpCodesToString); + attachDebugGetter(remove, i18nRemoveOpCodesToString); attachDebugGetter(update, i18nUpdateOpCodesToString); } tIcu.cases.push(caseName); @@ -611,7 +611,7 @@ export function parseIcuCase( function walkIcuTree( tView: TView, tIcu: TIcu, lView: LView, sharedUpdateOpCodes: I18nUpdateOpCodes, - create: I18nMutateOpCodes, remove: I18nMutateOpCodes, update: I18nUpdateOpCodes, + create: IcuCreateOpCodes, remove: I18nRemoveOpCodes, update: I18nUpdateOpCodes, parentNode: Element, parentIdx: number, nestedIcus: IcuExpression[], depth: number): number { let bindingMask = 0; let currentNode = parentNode.firstChild; @@ -648,9 +648,7 @@ function walkIcuTree( } (see http://g.co/ng/security#xss)`); } } else { - create.push( - newIndex << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Attr, attr.name, - attr.value); + addCreateAttribute(create, newIndex, attr); } } // Parse the children of this node (if any) @@ -689,16 +687,17 @@ function walkIcuTree( } return bindingMask; } -function addRemoveNode(remove: I18nMutateOpCodes, index: number, depth: number) { + +function addRemoveNode(remove: I18nRemoveOpCodes, index: number, depth: number) { if (depth === 0) { - remove.push(index << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Remove); + remove.push(index); } } -function addRemoveNestedIcu(remove: I18nMutateOpCodes, index: number, depth: number) { +function addRemoveNestedIcu(remove: I18nRemoveOpCodes, index: number, depth: number) { if (depth === 0) { - remove.push(index << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.RemoveNestedIcu); - remove.push(index << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Remove); + remove.push(~index); // remove ICU at `index` + remove.push(index); // remove ICU comment at `index` } } @@ -714,12 +713,16 @@ function addUpdateIcuUpdate(update: I18nUpdateOpCodes, bindingMask: number, inde } function addCreateNodeAndAppend( - create: I18nMutateOpCodes, marker: null|ICU_MARKER|ELEMENT_MARKER, text: string, + create: IcuCreateOpCodes, marker: null|ICU_MARKER|ELEMENT_MARKER, text: string, appendToParentIdx: number, createAtIdx: number) { if (marker !== null) { create.push(marker); } create.push( text, createAtIdx, - i18nMutateOpCode(I18nMutateOpCode.AppendChild, appendToParentIdx, createAtIdx)); + icuCreateOpCode(IcuCreateOpCode.AppendChild, appendToParentIdx, createAtIdx)); } + +function addCreateAttribute(create: IcuCreateOpCodes, newIndex: number, attr: Attr) { + create.push(newIndex << IcuCreateOpCode.SHIFT_REF | IcuCreateOpCode.Attr, attr.name, attr.value); +} \ No newline at end of file diff --git a/packages/core/src/render3/instructions/i18n_icu_container_visitor.ts b/packages/core/src/render3/instructions/i18n_icu_container_visitor.ts index fbfe11ac8e..d91bed33b0 100644 --- a/packages/core/src/render3/instructions/i18n_icu_container_visitor.ts +++ b/packages/core/src/render3/instructions/i18n_icu_container_visitor.ts @@ -6,10 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import {assertDomNode, assertEqual, assertNumber, assertNumberInRange} from '../../util/assert'; +import {assertDomNode, assertNumber, assertNumberInRange} from '../../util/assert'; import {assertTIcu, assertTNodeForLView} from '../assert'; import {EMPTY_ARRAY} from '../empty'; -import {getCurrentICUCaseIndex, I18nMutateOpCode, I18nMutateOpCodes, TIcu} from '../interfaces/i18n'; +import {getCurrentICUCaseIndex, I18nRemoveOpCodes, TIcu} from '../interfaces/i18n'; import {TIcuContainerNode} from '../interfaces/node'; import {RNode} from '../interfaces/renderer'; import {LView, TVIEW} from '../interfaces/view'; @@ -18,7 +18,7 @@ export function loadIcuContainerVisitor() { const _stack: any[] = []; let _index: number = -1; let _lView: LView; - let _removes: I18nMutateOpCodes; + let _removes: I18nRemoveOpCodes; /** * Retrieves a set of root nodes from `TIcu.remove`. Used by `TNodeType.ICUContainer` @@ -52,7 +52,7 @@ export function loadIcuContainerVisitor() { ngDevMode && assertNumberInRange(currentCase, 0, tIcu.cases.length - 1); _removes = tIcu.remove[currentCase]; } else { - _removes = EMPTY_ARRAY; + _removes = EMPTY_ARRAY as any; } } @@ -61,16 +61,15 @@ export function loadIcuContainerVisitor() { if (_index < _removes.length) { const removeOpCode = _removes[_index++] as number; ngDevMode && assertNumber(removeOpCode, 'Expecting OpCode number'); - const opCode = removeOpCode & I18nMutateOpCode.MASK_INSTRUCTION; - if (opCode === I18nMutateOpCode.Remove) { - const rNode = _lView[removeOpCode >>> I18nMutateOpCode.SHIFT_REF]; + if (removeOpCode > 0) { + const rNode = _lView[removeOpCode]; ngDevMode && assertDomNode(rNode); return rNode; } else { - ngDevMode && - assertEqual(opCode, I18nMutateOpCode.RemoveNestedIcu, 'Expecting RemoveNestedIcu'); _stack.push(_index, _removes); - const tIcu = _lView[TVIEW].data[removeOpCode >>> I18nMutateOpCode.SHIFT_REF] as TIcu; + // ICUs are represented by negative indices + const tIcuIndex = ~removeOpCode; + const tIcu = _lView[TVIEW].data[tIcuIndex] as TIcu; ngDevMode && assertTIcu(tIcu); enterIcu(tIcu, _lView); return icuContainerIteratorNext(); diff --git a/packages/core/src/render3/interfaces/i18n.ts b/packages/core/src/render3/interfaces/i18n.ts index 4ae81e8e49..9a75505d31 100644 --- a/packages/core/src/render3/interfaces/i18n.ts +++ b/packages/core/src/render3/interfaces/i18n.ts @@ -12,6 +12,18 @@ import {RNode} from './renderer'; import {SanitizerFn} from './sanitization'; import {LView} from './view'; + +/** + * Stores a list of nodes which need to be removed. + * + * Numbers are indexes into the `LView` + * - index > 0: `removeRNode(lView[0])` + * - index < 0: `removeICU(~lView[0])` + */ +export interface I18nRemoveOpCodes extends Array { + __brand__: 'I18nRemoveOpCodes'; +} + /** * `I18nMutateOpCode` defines OpCodes for `I18nMutateOpCodes` array. * @@ -35,11 +47,11 @@ import {LView} from './view'; * * See: `I18nCreateOpCodes` for example of usage. */ -export const enum I18nMutateOpCode { +export const enum IcuCreateOpCode { /** * Stores shift amount for bits 17-3 that contain reference index. */ - SHIFT_REF = 3, + SHIFT_REF = 1, /** * Stores shift amount for bits 31-17 that contain parent index. */ @@ -47,61 +59,124 @@ export const enum I18nMutateOpCode { /** * Mask for OpCode */ - // FIXME(misko): Shrink mask to 2 bits as 4 choices can fit into two bits. - MASK_INSTRUCTION = 0b111, + MASK_INSTRUCTION = 0b1, /** * Mask for the Reference node (bits 16-3) */ - // FIXME(misko): Why is this not used? - MASK_REF = 0b11111111111111000, + MASK_REF = 0b11111111111111110, // 11111110000000000 // 65432109876543210 /** * Instruction to append the current node to `PARENT`. */ - AppendChild = 0b001, - - /** - * Instruction to remove the `REF` node from `PARENT`. - */ - Remove = 0b011, + AppendChild = 0b0, /** * Instruction to set the attribute of a node. */ - Attr = 0b100, + Attr = 0b1, +} + + +/** + * Array storing OpCode for dynamically creating `i18n` blocks. + * + * Example: + * ```ts + * [ + * // For adding text nodes + * // --------------------- + * // Equivalent to: + * // lView[1].appendChild(lView[0] = document.createTextNode('xyz')); + * 'xyz', 0, 1 << SHIFT_PARENT | 0 << SHIFT_REF | AppendChild, + * + * // For adding element nodes + * // --------------------- + * // Equivalent to: + * // lView[1].appendChild(lView[0] = document.createElement('div')); + * ELEMENT_MARKER, 'div', 0, 1 << SHIFT_PARENT | 0 << SHIFT_REF | AppendChild, + * + * // For adding comment nodes + * // --------------------- + * // Equivalent to: + * // lView[1].appendChild(lView[0] = document.createComment('')); + * ICU_MARKER, '', 0, 1 << SHIFT_PARENT | 0 << SHIFT_REF | AppendChild, + * + * // For moving existing nodes to a different location + * // -------------------------------------------------- + * // Equivalent to: + * // const node = lView[1]; + * // lView[2].appendChild(node); + * 1 << SHIFT_REF | Select, 2 << SHIFT_PARENT | 0 << SHIFT_REF | AppendChild, + * + * // For removing existing nodes + * // -------------------------------------------------- + * // const node = lView[1]; + * // removeChild(tView.data(1), node, lView); + * 1 << SHIFT_REF | Remove, + * + * // For writing attributes + * // -------------------------------------------------- + * // const node = lView[1]; + * // node.setAttribute('attr', 'value'); + * 1 << SHIFT_REF | Attr, 'attr', 'value' + * ]; + * ``` + */ +export interface IcuCreateOpCodes extends Array, + I18nDebug { + __brand__: 'I18nCreateOpCodes'; +} + +export const enum I18nUpdateOpCode { + /** + * Stores shift amount for bits 17-2 that contain reference index. + */ + SHIFT_REF = 2, + /** + * Mask for OpCode + */ + MASK_OPCODE = 0b11, /** - * Instruction to removed the nested ICU. + * Instruction to update a text node. */ - RemoveNestedIcu = 0b110, + Text = 0b00, + /** + * Instruction to update a attribute of a node. + */ + Attr = 0b01, + /** + * Instruction to switch the current ICU case. + */ + IcuSwitch = 0b10, + /** + * Instruction to update the current ICU case. + */ + IcuUpdate = 0b11, } // FIXME(misko): These function are technically not interfaces, and so we may consider moving them // elsewhere. -// FIXME(misko): rename to `getParentFromI18nCreateOpCode` -export function getParentFromI18nMutateOpCode(mergedCode: number): number { - return mergedCode >>> I18nMutateOpCode.SHIFT_PARENT; +export function getParentFromIcuCreateOpCode(mergedCode: number): number { + return mergedCode >>> IcuCreateOpCode.SHIFT_PARENT; } -// FIXME(misko): rename to `getRefFromI18nCreateOpCode` -export function getRefFromI18nMutateOpCode(mergedCode: number): number { - return (mergedCode & I18nMutateOpCode.MASK_REF) >>> I18nMutateOpCode.SHIFT_REF; +export function getRefFromIcuCreateOpCode(mergedCode: number): number { + return (mergedCode & IcuCreateOpCode.MASK_REF) >>> IcuCreateOpCode.SHIFT_REF; } -// FIXME(misko): rename to `getInstructionFromI18nCreateOpCode` -export function getInstructionFromI18nMutateOpCode(mergedCode: number): number { - return mergedCode & I18nMutateOpCode.MASK_INSTRUCTION; +export function getInstructionFromIcuCreateOpCode(mergedCode: number): number { + return mergedCode & IcuCreateOpCode.MASK_INSTRUCTION; } -// FIXME(misko): rename to `i18nCreateOpCode` -export function i18nMutateOpCode(opCode: I18nMutateOpCode, parentIdx: number, refIdx: number) { +export function icuCreateOpCode(opCode: IcuCreateOpCode, parentIdx: number, refIdx: number) { ngDevMode && assertGreaterThanOrEqual(parentIdx, 0, 'Missing parent index'); ngDevMode && assertGreaterThan(refIdx, 0, 'Missing ref index'); - return opCode | parentIdx << I18nMutateOpCode.SHIFT_PARENT | refIdx << I18nMutateOpCode.SHIFT_REF; + return opCode | parentIdx << IcuCreateOpCode.SHIFT_PARENT | refIdx << IcuCreateOpCode.SHIFT_REF; } /** @@ -173,7 +248,9 @@ export interface I18nDebug { * } * ``` */ -export interface I18nCreateOpCodes extends Array, I18nDebug {} +export interface I18nCreateOpCodes extends Array, I18nDebug { + __brand__: 'I18nCreateOpCodes'; +} /** * See `I18nCreateOpCodes` @@ -197,84 +274,6 @@ export enum I18nCreateOpCode { } -/** - * Array storing OpCode for dynamically creating `i18n` blocks. - * - * Example: - * ```ts - * [ - * // For adding text nodes - * // --------------------- - * // Equivalent to: - * // lView[1].appendChild(lView[0] = document.createTextNode('xyz')); - * 'xyz', 0, 1 << SHIFT_PARENT | 0 << SHIFT_REF | AppendChild, - * - * // For adding element nodes - * // --------------------- - * // Equivalent to: - * // lView[1].appendChild(lView[0] = document.createElement('div')); - * ELEMENT_MARKER, 'div', 0, 1 << SHIFT_PARENT | 0 << SHIFT_REF | AppendChild, - * - * // For adding comment nodes - * // --------------------- - * // Equivalent to: - * // lView[1].appendChild(lView[0] = document.createComment('')); - * ICU_MARKER, '', 0, 1 << SHIFT_PARENT | 0 << SHIFT_REF | AppendChild, - * - * // For moving existing nodes to a different location - * // -------------------------------------------------- - * // Equivalent to: - * // const node = lView[1]; - * // lView[2].appendChild(node); - * 1 << SHIFT_REF | Select, 2 << SHIFT_PARENT | 0 << SHIFT_REF | AppendChild, - * - * // For removing existing nodes - * // -------------------------------------------------- - * // const node = lView[1]; - * // removeChild(tView.data(1), node, lView); - * 1 << SHIFT_REF | Remove, - * - * // For writing attributes - * // -------------------------------------------------- - * // const node = lView[1]; - * // node.setAttribute('attr', 'value'); - * 1 << SHIFT_REF | Attr, 'attr', 'value' - * ]; - * ``` - * - * See: `applyI18nCreateOpCodes`; - */ -export interface I18nMutateOpCodes extends Array, - I18nDebug {} - -export const enum I18nUpdateOpCode { - /** - * Stores shift amount for bits 17-2 that contain reference index. - */ - SHIFT_REF = 2, - /** - * Mask for OpCode - */ - MASK_OPCODE = 0b11, - - /** - * Instruction to update a text node. - */ - Text = 0b00, - /** - * Instruction to update a attribute of a node. - */ - Attr = 0b01, - /** - * Instruction to switch the current ICU case. - */ - IcuSwitch = 0b10, - /** - * Instruction to update the current ICU case. - */ - IcuUpdate = 0b11, -} - /** * Stores DOM operations which need to be applied to update DOM render tree due to changes in * expressions. @@ -347,7 +346,9 @@ export const enum I18nUpdateOpCode { * ``` * */ -export interface I18nUpdateOpCodes extends Array, I18nDebug {} +export interface I18nUpdateOpCodes extends Array, I18nDebug { + __brand__: 'I18nUpdateOpCodes'; +} /** * Store information for the i18n translation block. @@ -410,14 +411,12 @@ export interface TIcu { /** * A set of OpCodes to apply in order to build up the DOM render tree for the ICU */ - // FIXME(misko): Rename `I18nMutateOpCodes` to `I18nCreateOpCodes`. - create: I18nMutateOpCodes[]; + create: IcuCreateOpCodes[]; /** * A set of OpCodes to apply in order to destroy the DOM render tree for the ICU. */ - // FIXME(misko): Rename `I18nMutateOpCodes` to `I18nRemoveOpCodes`. - remove: I18nMutateOpCodes[]; + remove: I18nRemoveOpCodes[]; /** * A set of OpCodes to apply in order to update the DOM render tree for the ICU bindings. diff --git a/packages/core/src/render3/util/debug_utils.ts b/packages/core/src/render3/util/debug_utils.ts index 518b0738f6..018b109167 100644 --- a/packages/core/src/render3/util/debug_utils.ts +++ b/packages/core/src/render3/util/debug_utils.ts @@ -31,7 +31,7 @@ export function attachDebugObject(obj: any, debug: any): void { * @param obj Object to patch * @param debugGetter Getter returning a value to patch */ -export function attachDebugGetter(obj: any, debugGetter: () => any): void { +export function attachDebugGetter(obj: T, debugGetter: (this: T) => any): void { if (ngDevMode) { Object.defineProperty(obj, 'debug', {get: debugGetter, enumerable: false}); } else { diff --git a/packages/core/test/render3/i18n/i18n_parse_spec.ts b/packages/core/test/render3/i18n/i18n_parse_spec.ts index c3745d48b0..a8bdfca305 100644 --- a/packages/core/test/render3/i18n/i18n_parse_spec.ts +++ b/packages/core/test/render3/i18n/i18n_parse_spec.ts @@ -10,7 +10,7 @@ import {ɵɵi18nApply, ɵɵi18nExp} from '@angular/core'; import {applyCreateOpCodes} from '@angular/core/src/render3/i18n/i18n_apply'; import {i18nStartFirstCreatePass} from '@angular/core/src/render3/i18n/i18n_parse'; import {getTIcu} from '@angular/core/src/render3/i18n/i18n_util'; -import {IcuType, TI18n} from '@angular/core/src/render3/interfaces/i18n'; +import {I18nUpdateOpCodes, IcuType, TI18n} from '@angular/core/src/render3/interfaces/i18n'; import {HEADER_OFFSET} from '@angular/core/src/render3/interfaces/view'; import {expect} from '@angular/core/testing/src/testing_internal'; import {matchTI18n, matchTIcu} from '../matchers'; @@ -29,7 +29,7 @@ describe('i18n_parse', () => { 'lView[22] = document.createText("some text");', 'parent.appendChild(lView[22]);', ]), - update: [], + update: [] as unknown as I18nUpdateOpCodes, })); fixture.apply(() => applyCreateOpCodes(fixture.lView, tI18n.create, fixture.host, null)); @@ -88,8 +88,8 @@ describe('i18n_parse', () => { matchDebug([]), ], remove: [ - matchDebug(['(lView[0] as Element).remove(lView[25])']), - matchDebug(['(lView[0] as Element).remove(lView[26])']) + matchDebug(['remove(lView[25])']), + matchDebug(['remove(lView[26])']), ], })); @@ -204,13 +204,13 @@ describe('i18n_parse', () => { ], remove: [ matchDebug([ - '(lView[0] as Element).remove(lView[26])', + 'remove(lView[26])', 'removeNestedICU(27)', - '(lView[0] as Element).remove(lView[27])', - '(lView[0] as Element).remove(lView[31])', + 'remove(lView[27])', + 'remove(lView[31])', ]), matchDebug([ - '(lView[0] as Element).remove(lView[32])', + 'remove(lView[32])', ]) ], })); @@ -237,8 +237,8 @@ describe('i18n_parse', () => { ]), ], remove: [ - matchDebug(['(lView[0] as Element).remove(lView[29])']), - matchDebug(['(lView[0] as Element).remove(lView[30])']) + matchDebug(['remove(lView[29])']), + matchDebug(['remove(lView[30])']), ], })); diff --git a/packages/core/test/render3/i18n/i18n_spec.ts b/packages/core/test/render3/i18n/i18n_spec.ts index 9bcd7e6459..fa4debd43b 100644 --- a/packages/core/test/render3/i18n/i18n_spec.ts +++ b/packages/core/test/render3/i18n/i18n_spec.ts @@ -86,7 +86,7 @@ describe('Runtime i18n', () => { `lView[${HEADER_OFFSET + 1}] = document.createText("simple text");`, `parent.appendChild(lView[${HEADER_OFFSET + 1}]);`, ]), - update: [], + update: [] as unknown as I18nUpdateOpCodes, }); }); @@ -113,7 +113,7 @@ describe('Runtime i18n', () => { `lView[${HEADER_OFFSET + 8}] = document.createText("!");`, `parent.appendChild(lView[${HEADER_OFFSET + 8}]);`, ]), - update: [], + update: [] as unknown as I18nUpdateOpCodes, }); }); @@ -212,7 +212,7 @@ describe('Runtime i18n', () => { `lView[${HEADER_OFFSET + 3}] = document.createText("before");`, `lView[${HEADER_OFFSET + 4}] = document.createText("after");`, ]), - update: [], + update: [] as unknown as I18nUpdateOpCodes, }); @@ -229,7 +229,7 @@ describe('Runtime i18n', () => { create: matchDebug([ `lView[${HEADER_OFFSET + 2}] = document.createText("middle");`, ]), - update: [], + update: [] as unknown as I18nUpdateOpCodes, }); }); @@ -293,17 +293,17 @@ describe('Runtime i18n', () => { ], remove: [ matchDebug([ - '(lView[0] as Element).remove(lView[24])', - '(lView[0] as Element).remove(lView[25])', - '(lView[0] as Element).remove(lView[27])', + 'remove(lView[24])', + 'remove(lView[25])', + 'remove(lView[27])', ]), matchDebug([ - '(lView[0] as Element).remove(lView[28])', - '(lView[0] as Element).remove(lView[29])', + 'remove(lView[28])', + 'remove(lView[29])', ]), matchDebug([ - '(lView[0] as Element).remove(lView[31])', - '(lView[0] as Element).remove(lView[32])', + 'remove(lView[31])', + 'remove(lView[32])', ]), ], update: [ @@ -372,13 +372,13 @@ describe('Runtime i18n', () => { ], remove: [ matchDebug([ - '(lView[0] as Element).remove(lView[24])', + 'remove(lView[24])', ]), matchDebug([ - '(lView[0] as Element).remove(lView[25])', + 'remove(lView[25])', 'removeNestedICU(26)', - '(lView[0] as Element).remove(lView[26])', - '(lView[0] as Element).remove(lView[31])', + 'remove(lView[26])', + 'remove(lView[31])', ]), ], }); @@ -407,9 +407,9 @@ describe('Runtime i18n', () => { matchDebug([]), ], remove: [ - matchDebug(['(lView[0] as Element).remove(lView[28])']), - matchDebug(['(lView[0] as Element).remove(lView[29])']), - matchDebug(['(lView[0] as Element).remove(lView[30])']) + matchDebug(['remove(lView[28])']), + matchDebug(['remove(lView[29])']), + matchDebug(['remove(lView[30])']), ], }); }); diff --git a/packages/core/test/render3/i18n_debug_spec.ts b/packages/core/test/render3/i18n_debug_spec.ts index 9a05808680..5d1f2d7766 100644 --- a/packages/core/test/render3/i18n_debug_spec.ts +++ b/packages/core/test/render3/i18n_debug_spec.ts @@ -6,13 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ -import {i18nCreateOpCodesToString, i18nMutateOpCodesToString, i18nUpdateOpCodesToString} from '@angular/core/src/render3/i18n/i18n_debug'; -import {ELEMENT_MARKER, I18nCreateOpCode, I18nMutateOpCode, I18nUpdateOpCode, ICU_MARKER} from '@angular/core/src/render3/interfaces/i18n'; +import {i18nCreateOpCodesToString, i18nRemoveOpCodesToString, i18nUpdateOpCodesToString, icuCreateOpCodesToString} from '@angular/core/src/render3/i18n/i18n_debug'; +import {ELEMENT_MARKER, I18nCreateOpCode, I18nCreateOpCodes, I18nRemoveOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, IcuCreateOpCode} from '@angular/core/src/render3/interfaces/i18n'; describe('i18n debug', () => { describe('i18nUpdateOpCodesToString', () => { it('should print nothing', () => { - expect(i18nUpdateOpCodesToString([])).toEqual([]); + expect(i18nUpdateOpCodesToString([] as unknown as I18nUpdateOpCodes)).toEqual([]); }); it('should print text opCode', () => { @@ -23,7 +23,7 @@ describe('i18n debug', () => { -4, ' post', 1 << I18nUpdateOpCode.SHIFT_REF | I18nUpdateOpCode.Text, - ])) + ] as unknown as I18nUpdateOpCodes)) .toEqual( ['if (mask & 0b11) { (lView[1] as Text).textContent = `pre ${lView[i-4]} post`; }']); }); @@ -39,8 +39,8 @@ describe('i18n debug', () => { 'pre ', -4, ' in ', -3, ' post', 1 << I18nUpdateOpCode.SHIFT_REF | I18nUpdateOpCode.Attr, - 'title', (v) => v, - ])) + 'title', (v: any) => v, + ] as unknown as I18nUpdateOpCodes)) .toEqual([ 'if (mask & 0b1) { (lView[1] as Element).setAttribute(\'title\', `pre ${lView[i-4]} in ${lView[i-3]} post`); }', 'if (mask & 0b10) { (lView[1] as Element).setAttribute(\'title\', (function (v) { return v; })(`pre ${lView[i-4]} in ${lView[i-3]} post`)); }' @@ -48,29 +48,31 @@ describe('i18n debug', () => { }); it('should print icuSwitch opCode', () => { - expect(i18nUpdateOpCodesToString([ - 0b100, 2, -5, 12 << I18nUpdateOpCode.SHIFT_REF | I18nUpdateOpCode.IcuSwitch - ])).toEqual(['if (mask & 0b100) { icuSwitchCase(12, `${lView[i-5]}`); }']); + expect(i18nUpdateOpCodesToString( + [0b100, 2, -5, 12 << I18nUpdateOpCode.SHIFT_REF | I18nUpdateOpCode.IcuSwitch] as + unknown as I18nUpdateOpCodes)) + .toEqual(['if (mask & 0b100) { icuSwitchCase(12, `${lView[i-5]}`); }']); }); it('should print icuUpdate opCode', () => { - expect(i18nUpdateOpCodesToString([ - 0b1000, 1, 13 << I18nUpdateOpCode.SHIFT_REF | I18nUpdateOpCode.IcuUpdate - ])).toEqual(['if (mask & 0b1000) { icuUpdateCase(13); }']); + expect(i18nUpdateOpCodesToString( + [0b1000, 1, 13 << I18nUpdateOpCode.SHIFT_REF | I18nUpdateOpCode.IcuUpdate] as + unknown as I18nUpdateOpCodes)) + .toEqual(['if (mask & 0b1000) { icuUpdateCase(13); }']); }); }); describe('i18nMutateOpCodesToString', () => { it('should print nothing', () => { - expect(i18nMutateOpCodesToString([])).toEqual([]); + expect(icuCreateOpCodesToString([] as unknown as I18nCreateOpCodes)).toEqual([]); }); it('should print text AppendChild', () => { - expect(i18nMutateOpCodesToString([ + expect(icuCreateOpCodesToString([ 'xyz', 0, - 1 << I18nMutateOpCode.SHIFT_PARENT | 0 << I18nMutateOpCode.SHIFT_REF | - I18nMutateOpCode.AppendChild - ])) + 1 << IcuCreateOpCode.SHIFT_PARENT | 0 << IcuCreateOpCode.SHIFT_REF | + IcuCreateOpCode.AppendChild + ] as unknown as I18nCreateOpCodes)) .toEqual([ 'lView[0] = document.createTextNode("xyz")', '(lView[1] as Element).appendChild(lView[0])' @@ -79,11 +81,11 @@ describe('i18n debug', () => { it('should print element AppendChild', () => { - expect(i18nMutateOpCodesToString([ + expect(icuCreateOpCodesToString([ ELEMENT_MARKER, 'xyz', 0, - 1 << I18nMutateOpCode.SHIFT_PARENT | 0 << I18nMutateOpCode.SHIFT_REF | - I18nMutateOpCode.AppendChild - ])) + 1 << IcuCreateOpCode.SHIFT_PARENT | 0 << IcuCreateOpCode.SHIFT_REF | + IcuCreateOpCode.AppendChild + ] as unknown as I18nCreateOpCodes)) .toEqual([ 'lView[0] = document.createElement("xyz")', '(lView[1] as Element).appendChild(lView[0])' @@ -91,11 +93,11 @@ describe('i18n debug', () => { }); it('should print comment AppendChild', () => { - expect(i18nMutateOpCodesToString([ + expect(icuCreateOpCodesToString([ ICU_MARKER, 'xyz', 0, - 1 << I18nMutateOpCode.SHIFT_PARENT | 0 << I18nMutateOpCode.SHIFT_REF | - I18nMutateOpCode.AppendChild - ])) + 1 << IcuCreateOpCode.SHIFT_PARENT | 0 << IcuCreateOpCode.SHIFT_REF | + IcuCreateOpCode.AppendChild + ] as unknown as I18nCreateOpCodes)) .toEqual([ 'lView[0] = document.createComment("xyz")', '(lView[1] as Element).appendChild(lView[0])' @@ -103,28 +105,28 @@ describe('i18n debug', () => { }); it('should print Remove', () => { - expect(i18nMutateOpCodesToString([ - 2 << I18nMutateOpCode.SHIFT_PARENT | 0 << I18nMutateOpCode.SHIFT_REF | - I18nMutateOpCode.Remove - ])).toEqual(['(lView[2] as Element).remove(lView[0])']); + expect(i18nRemoveOpCodesToString([123] as unknown as I18nRemoveOpCodes)).toEqual([ + 'remove(lView[123])' + ]); }); it('should print Attr', () => { - expect(i18nMutateOpCodesToString([ - 1 << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Attr, 'attr', 'value' - ])).toEqual(['(lView[1] as Element).setAttribute("attr", "value")']); + expect(icuCreateOpCodesToString( + [1 << IcuCreateOpCode.SHIFT_REF | IcuCreateOpCode.Attr, 'attr', 'value'] as + unknown as I18nCreateOpCodes)) + .toEqual(['(lView[1] as Element).setAttribute("attr", "value")']); }); it('should print RemoveNestedIcu', () => { - expect(i18nMutateOpCodesToString([ - 1 << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.RemoveNestedIcu, - ])).toEqual(['removeNestedICU(1)']); + expect(i18nRemoveOpCodesToString([~123] as unknown as I18nRemoveOpCodes)).toEqual([ + 'removeNestedICU(123)' + ]); }); }); describe('i18nCreateOpCodesToString', () => { it('should print nothing', () => { - expect(i18nCreateOpCodesToString([])).toEqual([]); + expect(i18nCreateOpCodesToString([] as unknown as I18nCreateOpCodes)).toEqual([]); }); it('should print text/comment creation', () => { @@ -134,7 +136,7 @@ describe('i18n debug', () => { 12 << I18nCreateOpCode.SHIFT | I18nCreateOpCode.COMMENT, 'comment at 12', // 13 << I18nCreateOpCode.SHIFT | I18nCreateOpCode.COMMENT | I18nCreateOpCode.APPEND_EAGERLY, 'comment at 13, append', // - ])) + ] as unknown as I18nCreateOpCodes)) .toEqual([ 'lView[10] = document.createText("text at 10");', 'lView[11] = document.createText("text at 11, append");', diff --git a/packages/core/test/render3/matchers.ts b/packages/core/test/render3/matchers.ts index 7e15888d10..8fefab7515 100644 --- a/packages/core/test/render3/matchers.ts +++ b/packages/core/test/render3/matchers.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {I18nDebug, I18nMutateOpCodes, TI18n, TIcu} from '@angular/core/src/render3/interfaces/i18n'; +import {I18nDebug, IcuCreateOpCodes, TI18n, TIcu} from '@angular/core/src/render3/interfaces/i18n'; import {TNode} from '@angular/core/src/render3/interfaces/node'; import {TView} from '@angular/core/src/render3/interfaces/view'; @@ -233,7 +233,7 @@ export function matchDomText(expectedText: string|undefined = undefined): } export function matchI18nMutableOpCodes(expectedMutableOpCodes: string[]): - jasmine.AsymmetricMatcher { + jasmine.AsymmetricMatcher { const matcher = function() {}; let _actual: any = null;