From a4e749ffca5b1f726c365cecaf0f5c4f13eec8d9 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 was as `markForCheck` (#49711) 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 #49711 --- packages/core/src/render3/component_ref.ts | 10 +++-- .../animations/bundle.golden_symbols.json | 3 -- .../forms_reactive/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 ++++++++++++++++++- 6 files changed, 49 insertions(+), 17 deletions(-) diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 4fc24a50140..9d20f93d0fa 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -33,10 +33,11 @@ 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 {markViewDirty} from './instructions/mark_view_dirty'; +import {addToViewTree, createLView, createTView, executeContentQueries, getOrCreateComponentTView, getOrCreateTNode, initializeDirectives, invokeDirectivesHostBindings, locateHostElement, markAsComponentHost, 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'; +import {Renderer} from './interfaces/renderer'; import {RElement, RNode} from './interfaces/renderer_dom'; import {CONTEXT, HEADER_OFFSET, INJECTOR, LView, LViewEnvironment, LViewFlags, TVIEW, TViewType} from './interfaces/view'; import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces'; @@ -47,7 +48,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 { @@ -291,7 +292,8 @@ export class ComponentRef extends AbstractComponentRef { const lView = this._rootLView; setInputsForProperty(lView[TVIEW], lView, dataValue, name, value); this.previousInputValues.set(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 eb0da64a409..fbbd31d8b7a 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -1265,9 +1265,6 @@ { "name": "markAsComponentHost" }, - { - "name": "markDirtyIfOnPush" - }, { "name": "markViewDirty" }, 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 e1254e7bdfe..f7b27f2ca2d 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -1376,9 +1376,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 90ad0242c07..4ba0e163e1c 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 @@ -1334,9 +1334,6 @@ { "name": "markAsComponentHost" }, - { - "name": "markDirtyIfOnPush" - }, { "name": "markDuplicates" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 09731494085..b6f7ddf240f 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -1193,9 +1193,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 adb3c24f2ea..8c752499830 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'; @@ -420,5 +421,46 @@ describe('ComponentFactory', () => { fixture.detectChanges(); expect(log).toEqual(['1', '2']); }); + + 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.innerText).toBe('1'); + + fixture.componentInstance.setInput('2'); + fixture.detectChanges(); + expect(fixture.nativeElement.innerText).toBe('2'); + }); }); });