From 24b57d8b41548d524036fc0dc12cec746c4d9342 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Mon, 9 Nov 2020 20:05:23 -0800 Subject: [PATCH] refactor(core): Cleanup circular dependency between ViewEngine and Ivy `ChangeDetectorRef`. (#39621) `ChangeDetectorRef` is declared in ViewEngine but it sub-classed in Ivy. This creates a circular dependency between ViewEngine `ChangeDetectorRef` which needs to declare `__NG_ELEMENT_ID__` and ivy factory which needs to create it. The workaround used to be to pass the `ChangeDetectorRef` through stack but that created a very convoluted code. This refactoring simply bundles the two files together and removes the stack workaround making the code simpler to follow. PR Close #39621 --- goldens/circular-deps/packages.json | 202 +++++++++++++----- .../change_detection/change_detector_ref.ts | 49 ++++- .../src/render3/view_engine_compatibility.ts | 38 +--- .../view_engine_compatibility_prebound.ts | 3 +- 4 files changed, 196 insertions(+), 96 deletions(-) diff --git a/goldens/circular-deps/packages.json b/goldens/circular-deps/packages.json index 0046c2aa6d..38ad9b0ce5 100644 --- a/goldens/circular-deps/packages.json +++ b/goldens/circular-deps/packages.json @@ -146,8 +146,26 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts", + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", + "packages/core/src/render3/interfaces/container.ts", + "packages/core/src/linker/view_ref.ts" + ], + [ + "packages/core/src/application_ref.ts", + "packages/core/src/application_tokens.ts", + "packages/core/src/linker/component_factory.ts", + "packages/core/src/change_detection/change_detection.ts", + "packages/core/src/change_detection/change_detector_ref.ts", + "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts", + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", + "packages/core/src/render3/interfaces/container.ts", "packages/core/src/render3/interfaces/view.ts", "packages/core/src/di/injector.ts", "packages/core/src/di/r3_injector.ts", @@ -160,8 +178,12 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts", + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", + "packages/core/src/render3/interfaces/container.ts", "packages/core/src/render3/interfaces/view.ts", "packages/core/src/metadata.ts", "packages/core/src/di.ts", @@ -181,20 +203,12 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/interfaces/node.ts", - "packages/core/src/render3/interfaces/view.ts", + "packages/core/src/render3/interfaces/renderer.ts", + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", "packages/core/src/render3/interfaces/container.ts", - "packages/core/src/linker/view_ref.ts" - ], - [ - "packages/core/src/application_ref.ts", - "packages/core/src/application_tokens.ts", - "packages/core/src/linker/component_factory.ts", - "packages/core/src/change_detection/change_detection.ts", - "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", - "packages/core/src/render3/interfaces/node.ts", "packages/core/src/render3/interfaces/view.ts", "packages/core/src/render3/interfaces/definition.ts", "packages/core/src/core.ts", @@ -216,6 +230,9 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", + "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts", + "packages/core/src/render/api.ts", "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/state.ts", "packages/core/src/render3/assert.ts", @@ -228,7 +245,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts" ], [ @@ -237,7 +253,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/di/injector.ts", @@ -251,7 +266,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/di.ts", @@ -267,7 +281,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/di.ts", @@ -282,7 +295,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/di.ts", @@ -295,7 +307,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/instructions/shared.ts", @@ -316,7 +327,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/instructions/shared.ts", @@ -340,7 +350,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/instructions/shared.ts", @@ -359,7 +368,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/instructions/shared.ts", @@ -379,7 +387,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/instructions/shared.ts", @@ -392,7 +399,6 @@ "packages/core/src/linker/component_factory.ts", "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/instructions/shared.ts", @@ -844,7 +850,6 @@ [ "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/linker/component_factory.ts" @@ -852,7 +857,6 @@ [ "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/instructions/shared.ts", @@ -864,7 +868,6 @@ [ "packages/core/src/change_detection/change_detection.ts", "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts", "packages/core/src/linker/view_container_ref.ts", "packages/core/src/render3/instructions/shared.ts", @@ -877,19 +880,16 @@ ], [ "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts" - ], - [ - "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/interfaces/node.ts", - "packages/core/src/render3/interfaces/view.ts", + "packages/core/src/render3/interfaces/renderer.ts", + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", "packages/core/src/render3/interfaces/container.ts", "packages/core/src/linker/view_ref.ts" ], [ "packages/core/src/change_detection/change_detector_ref.ts", - "packages/core/src/render3/view_engine_compatibility.ts", "packages/core/src/render3/view_ref.ts" ], [ @@ -998,31 +998,129 @@ "packages/core/src/render3/view_engine_compatibility.ts" ], [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", "packages/core/src/render3/interfaces/container.ts", "packages/core/src/render3/interfaces/node.ts", - "packages/core/src/render3/interfaces/view.ts" + "packages/core/src/render3/interfaces/renderer.ts" ], [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", "packages/core/src/render3/interfaces/container.ts", - "packages/core/src/render3/interfaces/view.ts" + "packages/core/src/render3/interfaces/renderer.ts" ], [ - "packages/core/src/render3/interfaces/definition.ts", - "packages/core/src/render3/interfaces/node.ts", - "packages/core/src/render3/interfaces/view.ts" - ], - [ - "packages/core/src/render3/interfaces/definition.ts", - "packages/core/src/render3/interfaces/view.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/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", + "packages/core/src/render3/interfaces/container.ts", "packages/core/src/render3/interfaces/view.ts", - "packages/core/src/render3/interfaces/query.ts" + "packages/core/src/render3/interfaces/definition.ts", + "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", + "packages/core/src/render3/interfaces/container.ts", + "packages/core/src/render3/interfaces/view.ts", + "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", + "packages/core/src/render3/interfaces/container.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/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", + "packages/core/src/render3/interfaces/container.ts", + "packages/core/src/render3/interfaces/view.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", + "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/interfaces/type_checks.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/state.ts", + "packages/core/src/render3/assert.ts", + "packages/core/src/render3/interfaces/injector.ts", + "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/state.ts", + "packages/core/src/render3/assert.ts", + "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/state.ts", + "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/state.ts", + "packages/core/src/render3/util/view_utils.ts", + "packages/core/src/render3/interfaces/context.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/state.ts", + "packages/core/src/render3/util/view_utils.ts", + "packages/core/src/render3/interfaces/node.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render/api.ts", + "packages/core/src/render3/view_engine_compatibility.ts", + "packages/core/src/render3/state.ts", + "packages/core/src/render3/util/view_utils.ts", + "packages/core/src/render3/interfaces/renderer.ts" + ], + [ + "packages/core/src/render3/interfaces/container.ts", + "packages/core/src/render3/interfaces/view.ts" + ], + [ + "packages/core/src/render3/interfaces/definition.ts", + "packages/core/src/render3/interfaces/view.ts" ], [ "packages/core/src/render3/interfaces/query.ts", diff --git a/packages/core/src/change_detection/change_detector_ref.ts b/packages/core/src/change_detection/change_detector_ref.ts index 43a67707e6..f467d4faf8 100644 --- a/packages/core/src/change_detection/change_detector_ref.ts +++ b/packages/core/src/change_detection/change_detector_ref.ts @@ -6,7 +6,18 @@ * found in the LICENSE file at https://angular.io/license */ -import {injectChangeDetectorRef as render3InjectChangeDetectorRef} from '../render3/view_engine_compatibility'; +import {TNode, TNodeType} from '../render3/interfaces/node'; +import {isComponentHost} from '../render3/interfaces/type_checks'; +import {DECLARATION_COMPONENT_VIEW, LView} from '../render3/interfaces/view'; +import {getCurrentTNode, getLView} from '../render3/state'; +import {getComponentLViewByIndex} from '../render3/util/view_utils'; +import {ViewRef as R3_ViewRef} from '../render3/view_ref'; +import {noop} from '../util/noop'; + +export const SWITCH_CHANGE_DETECTOR_REF_FACTORY__POST_R3__ = injectChangeDetectorRef; +const SWITCH_CHANGE_DETECTOR_REF_FACTORY__PRE_R3__ = noop; +const SWITCH_CHANGE_DETECTOR_REF_FACTORY: typeof injectChangeDetectorRef = + SWITCH_CHANGE_DETECTOR_REF_FACTORY__PRE_R3__; /** * Base class that provides change detection functionality. @@ -114,12 +125,38 @@ export abstract class ChangeDetectorRef { * @internal * @nocollapse */ - static __NG_ELEMENT_ID__: () => ChangeDetectorRef = () => SWITCH_CHANGE_DETECTOR_REF_FACTORY(); + static __NG_ELEMENT_ID__: () => ChangeDetectorRef = SWITCH_CHANGE_DETECTOR_REF_FACTORY; } -export const SWITCH_CHANGE_DETECTOR_REF_FACTORY__POST_R3__ = render3InjectChangeDetectorRef; -const SWITCH_CHANGE_DETECTOR_REF_FACTORY__PRE_R3__ = (...args: any[]): any => {}; -const SWITCH_CHANGE_DETECTOR_REF_FACTORY: typeof render3InjectChangeDetectorRef = - SWITCH_CHANGE_DETECTOR_REF_FACTORY__PRE_R3__; +/** Returns a ChangeDetectorRef (a.k.a. a ViewRef) */ +export function injectChangeDetectorRef(isPipe = false): ChangeDetectorRef { + return createViewRef(getCurrentTNode()!, getLView(), isPipe); +} + +/** + * Creates a ViewRef and stores it on the injector as ChangeDetectorRef (public alias). + * + * @param tNode The node that is requesting a ChangeDetectorRef + * @param lView The view to which the node belongs + * @param isPipe Whether the view is being injected into a pipe. + * @returns The ChangeDetectorRef to use + */ +function createViewRef(tNode: TNode, lView: LView, isPipe: boolean): ChangeDetectorRef { + // `isComponentView` will be true for Component and Directives (but not for Pipes). + // See https://github.com/angular/angular/pull/33072 for proper fix + const isComponentView = !isPipe && isComponentHost(tNode); + if (isComponentView) { + // The LView represents the location where the component is declared. + // Instead we want the LView for the component View and so we need to look it up. + const componentView = getComponentLViewByIndex(tNode.index, lView); // look down + return new R3_ViewRef(componentView, componentView); + } else if (tNode.type & (TNodeType.AnyRNode | TNodeType.AnyContainer | TNodeType.Icu)) { + // The LView represents the location where the injection is requested from. + // We need to locate the containing LView (in case where the `lView` is an embedded view) + const hostComponentView = lView[DECLARATION_COMPONENT_VIEW]; // look up + return new R3_ViewRef(hostComponentView, lView); + } + return null!; +} \ No newline at end of file diff --git a/packages/core/src/render3/view_engine_compatibility.ts b/packages/core/src/render3/view_engine_compatibility.ts index 85eb4a7f3a..8d5d8cc414 100644 --- a/packages/core/src/render3/view_engine_compatibility.ts +++ b/packages/core/src/render3/view_engine_compatibility.ts @@ -6,49 +6,15 @@ * found in the LICENSE file at https://angular.io/license */ -import {ChangeDetectorRef as ViewEngine_ChangeDetectorRef} from '../change_detection/change_detector_ref'; import {Renderer2} from '../render/api'; -import {TNode, TNodeType} from './interfaces/node'; import {isProceduralRenderer} from './interfaces/renderer'; -import {isComponentHost, isLView} from './interfaces/type_checks'; -import {DECLARATION_COMPONENT_VIEW, LView, RENDERER} from './interfaces/view'; +import {isLView} from './interfaces/type_checks'; +import {LView, RENDERER} from './interfaces/view'; import {getCurrentTNode, getLView} from './state'; import {getComponentLViewByIndex} from './util/view_utils'; -import {ViewRef} from './view_ref'; -/** Returns a ChangeDetectorRef (a.k.a. a ViewRef) */ -export function injectChangeDetectorRef(isPipe = false): ViewEngine_ChangeDetectorRef { - return createViewRef(getCurrentTNode()!, getLView(), isPipe); -} - -/** - * Creates a ViewRef and stores it on the injector as ChangeDetectorRef (public alias). - * - * @param tNode The node that is requesting a ChangeDetectorRef - * @param lView The view to which the node belongs - * @param isPipe Whether the view is being injected into a pipe. - * @returns The ChangeDetectorRef to use - */ -function createViewRef(tNode: TNode, lView: LView, isPipe: boolean): ViewEngine_ChangeDetectorRef { - // `isComponentView` will be true for Component and Directives (but not for Pipes). - // See https://github.com/angular/angular/pull/33072 for proper fix - const isComponentView = !isPipe && isComponentHost(tNode); - if (isComponentView) { - // The LView represents the location where the component is declared. - // Instead we want the LView for the component View and so we need to look it up. - const componentView = getComponentLViewByIndex(tNode.index, lView); // look down - return new ViewRef(componentView, componentView); - } else if (tNode.type & (TNodeType.AnyRNode | TNodeType.AnyContainer | TNodeType.Icu)) { - // The LView represents the location where the injection is requested from. - // We need to locate the containing LView (in case where the `lView` is an embedded view) - const hostComponentView = lView[DECLARATION_COMPONENT_VIEW]; // look up - return new ViewRef(hostComponentView, lView); - } - return null!; -} - /** Returns a Renderer2 (or throws when application was bootstrapped with Renderer3) */ function getOrCreateRenderer2(view: LView): Renderer2 { const renderer = view[RENDERER]; diff --git a/packages/core/src/render3/view_engine_compatibility_prebound.ts b/packages/core/src/render3/view_engine_compatibility_prebound.ts index a4ade3b23b..2b748eef32 100644 --- a/packages/core/src/render3/view_engine_compatibility_prebound.ts +++ b/packages/core/src/render3/view_engine_compatibility_prebound.ts @@ -7,13 +7,12 @@ */ -import {ChangeDetectorRef} from '../change_detection/change_detector_ref'; +import {ChangeDetectorRef, injectChangeDetectorRef} from '../change_detection/change_detector_ref'; import {InjectFlags} from '../di/interface/injector'; import {createTemplateRef, TemplateRef} from '../linker/template_ref'; import {throwProviderNotFoundError} from './errors'; import {TNode} from './interfaces/node'; import {LView} from './interfaces/view'; -import {injectChangeDetectorRef} from './view_engine_compatibility'; /**