Skip to content

Commit

Permalink
fix(core): When using setInput, mark view dirty in same was as `markF…
Browse files Browse the repository at this point in the history
…orCheck` (#49711)

`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
  • Loading branch information
atscott authored and dylhunn committed Apr 5, 2023
1 parent aad05eb commit a4e749f
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 17 deletions.
10 changes: 6 additions & 4 deletions packages/core/src/render3/component_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 {
Expand Down Expand Up @@ -291,7 +292,8 @@ export class ComponentRef<T> extends AbstractComponentRef<T> {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1265,9 +1265,6 @@
{
"name": "markAsComponentHost"
},
{
"name": "markDirtyIfOnPush"
},
{
"name": "markViewDirty"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1376,9 +1376,6 @@
{
"name": "markAsComponentHost"
},
{
"name": "markDirtyIfOnPush"
},
{
"name": "markDuplicates"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1334,9 +1334,6 @@
{
"name": "markAsComponentHost"
},
{
"name": "markDirtyIfOnPush"
},
{
"name": "markDuplicates"
},
Expand Down
3 changes: 0 additions & 3 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -1193,9 +1193,6 @@
{
"name": "markAsComponentHost"
},
{
"name": "markDirtyIfOnPush"
},
{
"name": "markDuplicates"
},
Expand Down
44 changes: 43 additions & 1 deletion packages/core/test/render3/component_ref_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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: '<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.innerText).toBe('1');

fixture.componentInstance.setInput('2');
fixture.detectChanges();
expect(fixture.nativeElement.innerText).toBe('2');
});
});
});

0 comments on commit a4e749f

Please sign in to comment.