From 08f3d62391e51a5ac3564f25c95cc2bd4f713a04 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Sat, 17 Oct 2020 12:20:12 -0700 Subject: [PATCH] refactor(core): clean up circular dependencies (#39233) Moved code from `interfaces/i18n.ts` which was causing circular dependencies PR Close #39233 --- goldens/circular-deps/packages.json | 21 +------ packages/core/src/render3/i18n/i18n_apply.ts | 5 +- packages/core/src/render3/i18n/i18n_debug.ts | 4 +- packages/core/src/render3/i18n/i18n_parse.ts | 5 +- .../src/render3/i18n/i18n_tree_shaking.ts | 45 ++++++++++++++ packages/core/src/render3/i18n/i18n_util.ts | 38 +++++++++++- .../i18n_icu_container_visitor.ts | 3 +- packages/core/src/render3/interfaces/i18n.ts | 62 ------------------- .../core/src/render3/node_manipulation.ts | 2 +- packages/core/src/render3/view_ref.ts | 2 +- 10 files changed, 95 insertions(+), 92 deletions(-) create mode 100644 packages/core/src/render3/i18n/i18n_tree_shaking.ts diff --git a/goldens/circular-deps/packages.json b/goldens/circular-deps/packages.json index efe573fadd..8c26c0a489 100644 --- a/goldens/circular-deps/packages.json +++ b/goldens/circular-deps/packages.json @@ -223,7 +223,6 @@ "packages/core/src/render3/assert.ts", "packages/core/src/render3/interfaces/container.ts", "packages/core/src/render3/interfaces/node.ts", - "packages/core/src/render3/interfaces/i18n.ts", "packages/core/src/render3/interfaces/view.ts", "packages/core/src/di/injector.ts", "packages/core/src/di/r3_injector.ts", @@ -240,7 +239,6 @@ "packages/core/src/render3/assert.ts", "packages/core/src/render3/interfaces/container.ts", "packages/core/src/render3/interfaces/node.ts", - "packages/core/src/render3/interfaces/i18n.ts", "packages/core/src/render3/interfaces/view.ts", "packages/core/src/metadata.ts", "packages/core/src/di.ts", @@ -264,7 +262,6 @@ "packages/core/src/render3/assert.ts", "packages/core/src/render3/interfaces/container.ts", "packages/core/src/render3/interfaces/node.ts", - "packages/core/src/render3/interfaces/i18n.ts", "packages/core/src/render3/interfaces/view.ts", "packages/core/src/render3/interfaces/definition.ts", "packages/core/src/core.ts", @@ -971,13 +968,11 @@ [ "packages/core/src/render3/interfaces/container.ts", "packages/core/src/render3/interfaces/node.ts", - "packages/core/src/render3/interfaces/i18n.ts", "packages/core/src/render3/interfaces/view.ts" ], [ "packages/core/src/render3/interfaces/definition.ts", "packages/core/src/render3/interfaces/node.ts", - "packages/core/src/render3/interfaces/i18n.ts", "packages/core/src/render3/interfaces/view.ts" ], [ @@ -985,23 +980,13 @@ "packages/core/src/render3/interfaces/view.ts" ], [ - "packages/core/src/render3/interfaces/i18n.ts", - "packages/core/src/render3/interfaces/node.ts" - ], - [ - "packages/core/src/render3/interfaces/i18n.ts", + "packages/core/src/render3/interfaces/node.ts", "packages/core/src/render3/interfaces/view.ts" ], [ - "packages/core/src/render3/interfaces/i18n.ts", + "packages/core/src/render3/interfaces/node.ts", "packages/core/src/render3/interfaces/view.ts", - "packages/core/src/render3/interfaces/node.ts" - ], - [ - "packages/core/src/render3/interfaces/i18n.ts", - "packages/core/src/render3/interfaces/view.ts", - "packages/core/src/render3/interfaces/query.ts", - "packages/core/src/render3/interfaces/node.ts" + "packages/core/src/render3/interfaces/query.ts" ], [ "packages/core/src/render3/interfaces/query.ts", diff --git a/packages/core/src/render3/i18n/i18n_apply.ts b/packages/core/src/render3/i18n/i18n_apply.ts index f3b76d3ebe..7df72e9933 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, getParentFromIcuCreateOpCode, getRefFromIcuCreateOpCode, I18nCreateOpCode, I18nCreateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, IcuCreateOpCode, IcuCreateOpCodes, IcuType, TI18n, TIcu} from '../interfaces/i18n'; +import {ELEMENT_MARKER, 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'; @@ -20,9 +20,8 @@ import {createCommentNode, createElementNode, createTextNode, nativeInsertBefore import {getBindingIndex} from '../state'; import {renderStringify} from '../util/misc_utils'; import {getNativeByIndex, unwrapRNode} from '../util/view_utils'; - import {getLocaleId} from './i18n_locale_id'; -import {getTIcu} from './i18n_util'; +import {getCurrentICUCaseIndex, getParentFromIcuCreateOpCode, getRefFromIcuCreateOpCode, getTIcu} from './i18n_util'; diff --git a/packages/core/src/render3/i18n/i18n_debug.ts b/packages/core/src/render3/i18n/i18n_debug.ts index edac425025..cd3c8a6bbf 100644 --- a/packages/core/src/render3/i18n/i18n_debug.ts +++ b/packages/core/src/render3/i18n/i18n_debug.ts @@ -7,7 +7,9 @@ */ import {assertNumber, assertString} from '../../util/assert'; -import {ELEMENT_MARKER, getInstructionFromIcuCreateOpCode, getParentFromIcuCreateOpCode, getRefFromIcuCreateOpCode, I18nCreateOpCode, I18nCreateOpCodes, I18nRemoveOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, IcuCreateOpCode, IcuCreateOpCodes} from '../interfaces/i18n'; +import {ELEMENT_MARKER, I18nCreateOpCode, I18nCreateOpCodes, I18nRemoveOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, IcuCreateOpCode, IcuCreateOpCodes} from '../interfaces/i18n'; + +import {getInstructionFromIcuCreateOpCode, getParentFromIcuCreateOpCode, getRefFromIcuCreateOpCode} from './i18n_util'; /** diff --git a/packages/core/src/render3/i18n/i18n_parse.ts b/packages/core/src/render3/i18n/i18n_parse.ts index 49f9e2c8a1..2e4de13f35 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, I18nRemoveOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, icuCreateOpCode, IcuCreateOpCode, IcuCreateOpCodes, IcuExpression, IcuType, TI18n, TIcu} from '../interfaces/i18n'; +import {ELEMENT_MARKER, I18nCreateOpCode, I18nCreateOpCodes, I18nRemoveOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, ICU_MARKER, 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'; @@ -27,7 +27,8 @@ import {getNativeByIndex, getTNode} from '../util/view_utils'; import {i18nCreateOpCodesToString, i18nRemoveOpCodesToString, i18nUpdateOpCodesToString, icuCreateOpCodesToString} from './i18n_debug'; import {addTNodeAndUpdateInsertBeforeIndex} from './i18n_insert_before_index'; -import {createTNodePlaceholder, setTIcu, setTNodeInsertBeforeIndex} from './i18n_util'; +import {ensureIcuContainerVisitorLoaded} from './i18n_tree_shaking'; +import {createTNodePlaceholder, icuCreateOpCode, setTIcu, setTNodeInsertBeforeIndex} from './i18n_util'; diff --git a/packages/core/src/render3/i18n/i18n_tree_shaking.ts b/packages/core/src/render3/i18n/i18n_tree_shaking.ts new file mode 100644 index 0000000000..c1b5ee2537 --- /dev/null +++ b/packages/core/src/render3/i18n/i18n_tree_shaking.ts @@ -0,0 +1,45 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +/** + * @fileoverview + * + * This file provides mechanism by which code relevant to the `TIcuContainerNode` is only loaded if + * ICU is present in the template. + */ + +import {TIcuContainerNode} from '../interfaces/node'; +import {RNode} from '../interfaces/renderer'; +import {LView} from '../interfaces/view'; + + +let _icuContainerIterate: (tIcuContainerNode: TIcuContainerNode, lView: LView) => + (() => RNode | null); + +/** + * Iterator which provides ability to visit all of the `TIcuContainerNode` root `RNode`s. + */ +export function icuContainerIterate(tIcuContainerNode: TIcuContainerNode, lView: LView): () => + RNode | null { + return _icuContainerIterate(tIcuContainerNode, lView); +} + +/** + * Ensures that `IcuContainerVisitor`'s implementation is present. + * + * This function is invoked when i18n instruction comes across an ICU. The purpose is to allow the + * bundler to tree shake ICU logic and only load it if ICU instruction is executed. + */ +export function ensureIcuContainerVisitorLoaded( + loader: () => ((tIcuContainerNode: TIcuContainerNode, lView: LView) => (() => RNode | null))) { + if (_icuContainerIterate === undefined) { + // Do not inline this function. We want to keep `ensureIcuContainerVisitorLoaded` light, so it + // can be inlined into call-site. + _icuContainerIterate = loader(); + } +} diff --git a/packages/core/src/render3/i18n/i18n_util.ts b/packages/core/src/render3/i18n/i18n_util.ts index cfdd9ea77b..db4d7d6e70 100644 --- a/packages/core/src/render3/i18n/i18n_util.ts +++ b/packages/core/src/render3/i18n/i18n_util.ts @@ -6,12 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ -import {assertEqual, throwError} from '../../util/assert'; +import {assertEqual, assertGreaterThan, assertGreaterThanOrEqual, throwError} from '../../util/assert'; import {assertTIcu, assertTNode} from '../assert'; import {createTNodeAtIndex} from '../instructions/shared'; -import {TIcu} from '../interfaces/i18n'; +import {IcuCreateOpCode, TIcu} from '../interfaces/i18n'; import {TIcuContainerNode, TNode, TNodeType} from '../interfaces/node'; -import {TView} from '../interfaces/view'; +import {LView, TView} from '../interfaces/view'; import {assertTNodeType} from '../node_assert'; import {addTNodeAndUpdateInsertBeforeIndex} from './i18n_insert_before_index'; @@ -102,3 +102,35 @@ export function createTNodePlaceholder( addTNodeAndUpdateInsertBeforeIndex(previousTNodes, tNode); return tNode; } + + +/** + * Returns current ICU case. + * + * ICU cases are stored as index into the `TIcu.cases`. + * At times it is necessary to communicate that the ICU case just switched and that next ICU update + * should update all bindings regardless of the mask. In such a case the we store negative numbers + * for cases which have just been switched. This function removes the negative flag. + */ +export function getCurrentICUCaseIndex(tIcu: TIcu, lView: LView) { + const currentCase: number|null = lView[tIcu.currentCaseLViewIndex]; + return currentCase === null ? currentCase : (currentCase < 0 ? ~currentCase : currentCase); +} + +export function getParentFromIcuCreateOpCode(mergedCode: number): number { + return mergedCode >>> IcuCreateOpCode.SHIFT_PARENT; +} + +export function getRefFromIcuCreateOpCode(mergedCode: number): number { + return (mergedCode & IcuCreateOpCode.MASK_REF) >>> IcuCreateOpCode.SHIFT_REF; +} + +export function getInstructionFromIcuCreateOpCode(mergedCode: number): number { + return mergedCode & IcuCreateOpCode.MASK_INSTRUCTION; +} + +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 << IcuCreateOpCode.SHIFT_PARENT | refIdx << IcuCreateOpCode.SHIFT_REF; +} 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 d91bed33b0..e408ab5ce7 100644 --- a/packages/core/src/render3/instructions/i18n_icu_container_visitor.ts +++ b/packages/core/src/render3/instructions/i18n_icu_container_visitor.ts @@ -9,7 +9,8 @@ import {assertDomNode, assertNumber, assertNumberInRange} from '../../util/assert'; import {assertTIcu, assertTNodeForLView} from '../assert'; import {EMPTY_ARRAY} from '../empty'; -import {getCurrentICUCaseIndex, I18nRemoveOpCodes, TIcu} from '../interfaces/i18n'; +import {getCurrentICUCaseIndex} from '../i18n/i18n_util'; +import {I18nRemoveOpCodes, TIcu} from '../interfaces/i18n'; import {TIcuContainerNode} from '../interfaces/node'; import {RNode} from '../interfaces/renderer'; import {LView, TVIEW} from '../interfaces/view'; diff --git a/packages/core/src/render3/interfaces/i18n.ts b/packages/core/src/render3/interfaces/i18n.ts index 9a75505d31..80164d0aab 100644 --- a/packages/core/src/render3/interfaces/i18n.ts +++ b/packages/core/src/render3/interfaces/i18n.ts @@ -6,11 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {assertGreaterThan, assertGreaterThanOrEqual} from '../../util/assert'; -import {TIcuContainerNode} from './node'; -import {RNode} from './renderer'; import {SanitizerFn} from './sanitization'; -import {LView} from './view'; /** @@ -158,27 +154,6 @@ export const enum I18nUpdateOpCode { IcuUpdate = 0b11, } -// FIXME(misko): These function are technically not interfaces, and so we may consider moving them -// elsewhere. - -export function getParentFromIcuCreateOpCode(mergedCode: number): number { - return mergedCode >>> IcuCreateOpCode.SHIFT_PARENT; -} - -export function getRefFromIcuCreateOpCode(mergedCode: number): number { - return (mergedCode & IcuCreateOpCode.MASK_REF) >>> IcuCreateOpCode.SHIFT_REF; -} - -export function getInstructionFromIcuCreateOpCode(mergedCode: number): number { - return mergedCode & IcuCreateOpCode.MASK_INSTRUCTION; -} - -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 << IcuCreateOpCode.SHIFT_PARENT | refIdx << IcuCreateOpCode.SHIFT_REF; -} - /** * Marks that the next string is an element name. * @@ -437,40 +412,3 @@ export interface IcuExpression { cases: string[]; values: (string|IcuExpression)[][]; } - -let _icuContainerIterate: (tIcuContainerNode: TIcuContainerNode, lView: LView) => - (() => RNode | null); - -/** - * Iterator which provides ability to visit all of the `TIcuContainerNode` root `RNode`s. - */ -export function icuContainerIterate(tIcuContainerNode: TIcuContainerNode, lView: LView): () => - RNode | null { - return _icuContainerIterate(tIcuContainerNode, lView); -} - -/** - * Ensures that `IcuContainerVisitor`'s implementation is present. - * - * This function is invoked when i18n instruction comes across an ICU. The purpose is to allow the - * bundler to tree shake ICU logic and only load it if ICU instruction is executed. - */ -export function ensureIcuContainerVisitorLoaded( - loader: () => ((tIcuContainerNode: TIcuContainerNode, lView: LView) => (() => RNode | null))) { - if (_icuContainerIterate === undefined) { - // Do not inline this function. We want to keep `ensureIcuContainerVisitorLoaded` light, so it - // can be inlined into call-site. - _icuContainerIterate = loader(); - } -} - - -/** - * Returns current ICU case. - * - * We store negative numbers for cases which have just been switched. This function removes that. - */ -export function getCurrentICUCaseIndex(tIcu: TIcu, lView: LView) { - const currentCase: number|null = lView[tIcu.currentCaseLViewIndex]; - return currentCase === null ? currentCase : (currentCase < 0 ? ~currentCase : currentCase); -} \ No newline at end of file diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 5d8157b528..275c16f811 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -13,9 +13,9 @@ import {assertDefined, assertDomNode, assertEqual, assertIndexInRange, assertStr import {assertLContainer, assertLView, assertTNodeForLView} from './assert'; import {attachPatchData} from './context_discovery'; +import {icuContainerIterate} from './i18n/i18n_tree_shaking'; import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS, NATIVE, unusedValueExportToPlacateAjd as unused1} from './interfaces/container'; import {ComponentDef} from './interfaces/definition'; -import {icuContainerIterate} from './interfaces/i18n'; import {NodeInjectorFactory} from './interfaces/injector'; import {TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeType, TProjectionNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node'; import {unusedValueExportToPlacateAjd as unused3} from './interfaces/projection'; diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index e31ffe5195..f7f4b916f9 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -11,10 +11,10 @@ import {ChangeDetectorRef as viewEngine_ChangeDetectorRef} from '../change_detec import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref'; import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEngine_InternalViewRef} from '../linker/view_ref'; import {assertDefined} from '../util/assert'; +import {icuContainerIterate} from './i18n/i18n_tree_shaking'; import {checkNoChangesInRootView, checkNoChangesInternal, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupWithContext} from './instructions/shared'; import {CONTAINER_HEADER_OFFSET} from './interfaces/container'; -import {icuContainerIterate} from './interfaces/i18n'; import {TElementNode, TIcuContainerNode, TNode, TNodeType} from './interfaces/node'; import {RNode} from './interfaces/renderer'; import {isLContainer} from './interfaces/type_checks';