From e4dcaa513e7d5ccd3a63edf6132792873f01f7c1 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 5 Dec 2022 12:47:46 +0100 Subject: [PATCH] fix(core): unable to inject ChangeDetectorRef inside host directives (#48355) When injecting the `ChangeDetectorRef` into a node that matches a component, we create a new ref using the component's LView. This breaks down for host directives, because they run before the component's LView has been created. These changes resolve the issue by creating the LView before creating the node injector for the directives. Fixes #48249. PR Close #48355 --- .../core/src/render3/instructions/shared.ts | 21 +++++----- .../test/acceptance/host_directives_spec.ts | 40 ++++++++++++++++++- .../animations/bundle.golden_symbols.json | 3 -- .../cyclic_import/bundle.golden_symbols.json | 6 +-- .../forms_reactive/bundle.golden_symbols.json | 3 -- .../bundle.golden_symbols.json | 3 -- .../router/bundle.golden_symbols.json | 3 -- .../bundling/todo/bundle.golden_symbols.json | 3 -- 8 files changed, 54 insertions(+), 28 deletions(-) diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index f55cdf1e0e5db..306188203a4cf 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -1142,6 +1142,16 @@ function instantiateAllDirectives( tView: TView, lView: LView, tNode: TDirectiveHostNode, native: RNode) { const start = tNode.directiveStart; const end = tNode.directiveEnd; + + // The component view needs to be created before creating the node injector + // since it is used to inject some special symbols like `ChangeDetectorRef`. + if (isComponentHost(tNode)) { + ngDevMode && assertTNodeType(tNode, TNodeType.AnyRNode); + addComponentLogic( + lView, tNode as TElementNode, + tView.data[start + tNode.componentOffset] as ComponentDef); + } + if (!tView.firstCreatePass) { getOrCreateNodeInjectorForNode(tNode, lView); } @@ -1151,13 +1161,6 @@ function instantiateAllDirectives( const initialInputs = tNode.initialInputs; for (let i = start; i < end; i++) { const def = tView.data[i] as DirectiveDef; - const isComponent = isComponentDef(def); - - if (isComponent) { - ngDevMode && assertTNodeType(tNode, TNodeType.AnyRNode); - addComponentLogic(lView, tNode as TElementNode, def as ComponentDef); - } - const directive = getNodeInjectable(lView, tView, i, tNode); attachPatchData(directive, lView); @@ -1165,9 +1168,9 @@ function instantiateAllDirectives( setInputsFromAttrs(lView, i - start, directive, def, tNode, initialInputs!); } - if (isComponent) { + if (isComponentDef(def)) { const componentView = getComponentLViewByIndex(tNode.index, lView); - componentView[CONTEXT] = directive; + componentView[CONTEXT] = getNodeInjectable(lView, tView, i, tNode); } } } diff --git a/packages/core/test/acceptance/host_directives_spec.ts b/packages/core/test/acceptance/host_directives_spec.ts index 828f7bd8e582b..85b6a951814d8 100644 --- a/packages/core/test/acceptance/host_directives_spec.ts +++ b/packages/core/test/acceptance/host_directives_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AfterViewChecked, AfterViewInit, Component, Directive, ElementRef, EventEmitter, forwardRef, inject, Inject, InjectionToken, Input, OnChanges, OnInit, Output, SimpleChanges, Type, ViewChild, ViewContainerRef} from '@angular/core'; +import {AfterViewChecked, AfterViewInit, ChangeDetectorRef, Component, Directive, ElementRef, EventEmitter, forwardRef, inject, Inject, InjectionToken, Input, OnChanges, OnInit, Output, SimpleChanges, Type, ViewChild, ViewContainerRef} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; @@ -919,6 +919,44 @@ describe('host directives', () => { expect(() => TestBed.createComponent(App)) .toThrowError(/NG0200: Circular dependency in DI detected for HostDir/); }); + + it('should inject a valid ChangeDetectorRef when attached to a component', () => { + type InternalChangeDetectorRef = ChangeDetectorRef&{_lView: unknown}; + + @Directive({standalone: true}) + class HostDir { + changeDetectorRef = inject(ChangeDetectorRef) as InternalChangeDetectorRef; + } + + @Component({selector: 'my-comp', hostDirectives: [HostDir], template: ''}) + class Comp { + changeDetectorRef = inject(ChangeDetectorRef) as InternalChangeDetectorRef; + } + + @Component({template: ''}) + class App { + @ViewChild(HostDir) hostDir!: HostDir; + @ViewChild(Comp) comp!: Comp; + } + + TestBed.configureTestingModule({declarations: [App, Comp]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + const hostDirectiveCdr = fixture.componentInstance.hostDir.changeDetectorRef; + const componentCdr = fixture.componentInstance.comp.changeDetectorRef; + + // We can't assert that the change detectors are the same by comparing + // them directly, because a new one is created each time. Instead of we + // compare that they're associated with the same LView. + expect(hostDirectiveCdr._lView).toBeTruthy(); + expect(componentCdr._lView).toBeTruthy(); + expect(hostDirectiveCdr._lView).toBe(componentCdr._lView); + expect(() => { + hostDirectiveCdr.markForCheck(); + hostDirectiveCdr.detectChanges(); + }).not.toThrow(); + }); }); describe('outputs', () => { diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index 8428e0ae89bbd..c4c2964b81edd 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -548,9 +548,6 @@ { "name": "addClass" }, - { - "name": "addComponentLogic" - }, { "name": "addPropertyAlias" }, diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index eab3aa9ebf4d8..ee0fb51216764 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -386,9 +386,6 @@ { "name": "_wrapInTimeout" }, - { - "name": "addComponentLogic" - }, { "name": "addPropertyAlias" }, @@ -743,6 +740,9 @@ { "name": "isComponentDef" }, + { + "name": "isComponentHost" + }, { "name": "isContentQueryHost" }, diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index 85d3f57fda964..e5e3b20fb88f3 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -551,9 +551,6 @@ { "name": "_wrapInTimeout" }, - { - "name": "addComponentLogic" - }, { "name": "addPropertyAlias" }, diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index 322393d2c42a7..d77e86ef2da1c 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -539,9 +539,6 @@ { "name": "_wrapInTimeout" }, - { - "name": "addComponentLogic" - }, { "name": "addPropertyAlias" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index 5849d985071e6..14c45f28a39a6 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -707,9 +707,6 @@ { "name": "absoluteRedirect" }, - { - "name": "addComponentLogic" - }, { "name": "addPropertyAlias" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 6b32a8b840355..c2c3cc82cfc3d 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -464,9 +464,6 @@ { "name": "_wrapInTimeout" }, - { - "name": "addComponentLogic" - }, { "name": "addPropertyAlias" },