From 702ec901100b2d84efdf0b16d8347f8b28b94d5d Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 4 Apr 2023 18:20:25 -0700 Subject: [PATCH] fix(core): When using setInput, mark view dirty in same way as `markForCheck` (#49747) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ComponentRef.setInput` internally calls `markDirtyIfOnPush` which only marks the given view as dirty but does not mark parents dirty like `ChangeDetectorRef.markForCheck` would. https://github.com/angular/angular/blob/f071224720f8affb97fd32fb5aeaa13155b13693/packages/core/src/render3/instructions/shared.ts#L1018-L1024 `markDirtyIfOnPush` has an assumption that it’s being called from the parent’s template. That is, we don’t need to mark dirty to the root, because we’ve already traversed down to it. The function used to only be called during template execution for input bindings but was added to `setInput` later. It's not a good fit because it means that if you are responding to events such as an emit from an `Observable` and call `setInput`, the view of your `ComponentRef` won't necessarily get checked when change detection runs next. If this lives inside some `OnPush` component tree that's not already dirty, it only gets refreshed if you also call `ChangeDetectorRef.markForCheck` in the host component (because it will be "shielded" be a non-dirty parent). PR Close #49747 --- packages/core/src/render3/component_ref.ts | 7 +-- .../animations/bundle.golden_symbols.json | 2 +- .../cyclic_import/bundle.golden_symbols.json | 3 ++ .../forms_reactive/bundle.golden_symbols.json | 3 -- .../bundle.golden_symbols.json | 3 -- .../hello_world/bundle.golden_symbols.json | 3 ++ .../bundle.golden_symbols.json | 3 ++ .../bundling/todo/bundle.golden_symbols.json | 3 -- .../core/test/render3/component_ref_spec.ts | 44 ++++++++++++++++++- 9 files changed, 57 insertions(+), 14 deletions(-) diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 4b3f884dc39..47250c0a08d 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -31,7 +31,7 @@ import {getNodeInjectable, NodeInjector} from './di'; import {throwProviderNotFoundError} from './errors_di'; import {registerPostOrderHooks} from './hooks'; import {reportUnknownPropertyError} from './instructions/element_validation'; -import {addToViewTree, createLView, createTView, executeContentQueries, getOrCreateComponentTView, getOrCreateTNode, initializeDirectives, invokeDirectivesHostBindings, locateHostElement, markAsComponentHost, markDirtyIfOnPush, renderView, setInputsForProperty} from './instructions/shared'; +import {addToViewTree, createLView, createTView, executeContentQueries, getOrCreateComponentTView, getOrCreateTNode, initializeDirectives, invokeDirectivesHostBindings, locateHostElement, markAsComponentHost, markViewDirty, renderView, setInputsForProperty} from './instructions/shared'; import {ComponentDef, DirectiveDef, HostDirectiveDefs} from './interfaces/definition'; import {PropertyAliasValue, TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType} from './interfaces/node'; import {Renderer, RendererFactory} from './interfaces/renderer'; @@ -44,7 +44,7 @@ import {enterView, getCurrentTNode, getLView, leaveView} from './state'; import {computeStaticStyling} from './styling/static_styling'; import {mergeHostAttrs, setUpAttributes} from './util/attrs_utils'; import {stringifyForError} from './util/stringify_utils'; -import {getNativeByTNode, getTNode} from './util/view_utils'; +import {getComponentLViewByIndex, getNativeByTNode, getTNode} from './util/view_utils'; import {RootViewRef, ViewRef} from './view_ref'; export class ComponentFactoryResolver extends AbstractComponentFactoryResolver { @@ -268,7 +268,8 @@ export class ComponentRef extends AbstractComponentRef { if (inputData !== null && (dataValue = inputData[name])) { const lView = this._rootLView; setInputsForProperty(lView[TVIEW], lView, dataValue, name, value); - markDirtyIfOnPush(lView, this._tNode.index); + const childComponentLView = getComponentLViewByIndex(this._tNode.index, lView); + markViewDirty(childComponentLView); } else { if (ngDevMode) { const cmpNameForError = stringifyForError(this.componentType); diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index f5db79c84db..cd0e4437bbf 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -1200,7 +1200,7 @@ "name": "markAsComponentHost" }, { - "name": "markDirtyIfOnPush" + "name": "markViewDirty" }, { "name": "maybeWrapInNotSelector" 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 96cff924706..ca6a929ae23 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -914,6 +914,9 @@ { "name": "markAsComponentHost" }, + { + "name": "markViewDirty" + }, { "name": "maybeWrapInNotSelector" }, 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 c524ab8b2f6..5ddf3d6d52c 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -1307,9 +1307,6 @@ { "name": "markAsComponentHost" }, - { - "name": "markDirtyIfOnPush" - }, { "name": "markDuplicates" }, 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 e9dc48d4d94..fcba035b4f7 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 @@ -1265,9 +1265,6 @@ { "name": "markAsComponentHost" }, - { - "name": "markDirtyIfOnPush" - }, { "name": "markDuplicates" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 88ef4362ee7..1d037f91fce 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -698,6 +698,9 @@ { "name": "makeRecord" }, + { + "name": "markViewDirty" + }, { "name": "maybeWrapInNotSelector" }, diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 0837d137917..baf14b89724 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -800,6 +800,9 @@ { "name": "makeRecord" }, + { + "name": "markViewDirty" + }, { "name": "maybeWrapInNotSelector" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 179f0ed5f2b..c274b4435f2 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -1112,9 +1112,6 @@ { "name": "markAsComponentHost" }, - { - "name": "markDirtyIfOnPush" - }, { "name": "markDuplicates" }, diff --git a/packages/core/test/render3/component_ref_spec.ts b/packages/core/test/render3/component_ref_spec.ts index e787a2c02c2..de5e1b56f0a 100644 --- a/packages/core/test/render3/component_ref_spec.ts +++ b/packages/core/test/render3/component_ref_spec.ts @@ -6,12 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ +import {ComponentRef} from '@angular/core'; import {ComponentFactoryResolver} from '@angular/core/src/render3/component_ref'; import {Renderer} from '@angular/core/src/render3/interfaces/renderer'; import {RElement} from '@angular/core/src/render3/interfaces/renderer_dom'; import {TestBed} from '@angular/core/testing'; -import {ChangeDetectionStrategy, Component, Injector, Input, NgModuleRef, OnChanges, Output, RendererType2, SimpleChanges, ViewEncapsulation} from '../../src/core'; +import {ChangeDetectionStrategy, Component, Injector, Input, NgModuleRef, OnChanges, Output, RendererType2, SimpleChanges, ViewChild, ViewContainerRef, ViewEncapsulation} from '../../src/core'; import {ComponentFactory} from '../../src/linker/component_factory'; import {RendererFactory2} from '../../src/render/api'; import {Sanitizer} from '../../src/sanitization/sanitizer'; @@ -397,5 +398,46 @@ describe('ComponentFactory', () => { fixture.detectChanges(); expect(fixture.nativeElement.textContent).toBe('pushed'); }); + + it('marks parents dirty so component is not "shielded" by a non-dirty OnPush parent', () => { + @Component({ + template: `{{input}}`, + standalone: true, + selector: 'dynamic', + }) + class DynamicCmp { + @Input() input?: string; + } + + @Component({ + template: '', + standalone: true, + imports: [DynamicCmp], + changeDetection: ChangeDetectionStrategy.OnPush, + }) + class Wrapper { + @ViewChild('template', {read: ViewContainerRef}) template?: ViewContainerRef; + componentRef?: ComponentRef; + + create() { + this.componentRef = this.template!.createComponent(DynamicCmp); + } + setInput(value: string) { + this.componentRef!.setInput('input', value); + } + } + + const fixture = TestBed.createComponent(Wrapper); + fixture.detectChanges(); + fixture.componentInstance.create(); + + fixture.componentInstance.setInput('1'); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toBe('1'); + + fixture.componentInstance.setInput('2'); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toBe('2'); + }); }); });