fix(core): When using setInput, mark view dirty in same way as markForCheck (#49747)

`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
This commit is contained in:
Andrew Scott
2023-04-04 18:20:25 -07:00
committed by Andrew Kushnir
parent ffdfdc238f
commit 702ec90110
9 changed files with 57 additions and 14 deletions
+4 -3
View File
@@ -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<T> extends AbstractComponentRef<T> {
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);
@@ -1200,7 +1200,7 @@
"name": "markAsComponentHost"
},
{
"name": "markDirtyIfOnPush"
"name": "markViewDirty"
},
{
"name": "maybeWrapInNotSelector"
@@ -914,6 +914,9 @@
{
"name": "markAsComponentHost"
},
{
"name": "markViewDirty"
},
{
"name": "maybeWrapInNotSelector"
},
@@ -1307,9 +1307,6 @@
{
"name": "markAsComponentHost"
},
{
"name": "markDirtyIfOnPush"
},
{
"name": "markDuplicates"
},
@@ -1265,9 +1265,6 @@
{
"name": "markAsComponentHost"
},
{
"name": "markDirtyIfOnPush"
},
{
"name": "markDuplicates"
},
@@ -698,6 +698,9 @@
{
"name": "makeRecord"
},
{
"name": "markViewDirty"
},
{
"name": "maybeWrapInNotSelector"
},
@@ -800,6 +800,9 @@
{
"name": "makeRecord"
},
{
"name": "markViewDirty"
},
{
"name": "maybeWrapInNotSelector"
},
@@ -1112,9 +1112,6 @@
{
"name": "markAsComponentHost"
},
{
"name": "markDirtyIfOnPush"
},
{
"name": "markDuplicates"
},
@@ -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: '<ng-template #template></ng-template>',
standalone: true,
imports: [DynamicCmp],
changeDetection: ChangeDetectionStrategy.OnPush,
})
class Wrapper {
@ViewChild('template', {read: ViewContainerRef}) template?: ViewContainerRef;
componentRef?: ComponentRef<DynamicCmp>;
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');
});
});
});