fix(core): ApplicationRef.tick should respect OnPush for host bindi…

…ngs (#53718) (#53718)

This commit updates `ApplicationRef.tick` to use `detectChangesInternal` for root
views rather than go through the `ChangeDetectorRef.detectChanges` API
which refreshes the host view without first looking at whether the view
is `OnPush` and not dirty. The current behavior would hide errors in
`OnPush` components that do not correctly get marked for check and would
break when migrating to zoneless change detection because `markForCheck`
was never called so change detection was never scheduled.
The error would be surprising and blamed on switching to zoneless when in
reality the issue already exists and is a problem with the component not
calling `markForCheck`. However, this error is hidden today because
`ApplicationRef.tick` refresh host bindings unconditionally.

BREAKING CHANGE: `OnPush` views at the root of the application need to
be marked dirty for their host bindings to refresh. Previously, the host
bindings were refreshed for all root views without respecting the
`OnPush` change detection strategy.

PR Close #53718

PR Close #53718

packages/core/src/application/application_ref.ts

@@ -722,24 +722,11 @@ function shouldRecheckView(view: LView): boolean {
 }
 
 function detectChangesInView(lView: LView, notifyErrorHandler: boolean, isFirstPass: boolean) {
-  let mode: ChangeDetectionMode;
-  if (isFirstPass) {
-    // The first pass is always in Global mode, which includes `CheckAlways` views.
-    mode = ChangeDetectionMode.Global;
-    // Add `RefreshView` flag to ensure this view is refreshed if not already dirty.
-    // `RefreshView` flag is used intentionally over `Dirty` because it gets cleared before
-    // executing any of the actual refresh code while the `Dirty` flag doesn't get cleared
-    // until the end of the refresh. Using `RefreshView` prevents creating a potential
-    // difference in the state of the LViewFlags during template execution.
-    lView[FLAGS] |= LViewFlags.RefreshView;
-  } else if (lView[FLAGS] & LViewFlags.Dirty) {
-    // The root view has been explicitly marked for check, so check it in Global mode.
-    mode = ChangeDetectionMode.Global;
-  } else {
-    // The view has not been marked for check, but contains a view marked for refresh
-    // (likely via a signal). Start this change detection in Targeted mode to skip the root
-    // view and check just the view(s) that need refreshed.
-    mode = ChangeDetectionMode.Targeted;
-  }
+  const mode = isFirstPass || lView[FLAGS] & LViewFlags.Dirty ?
+      // The first pass is always in Global mode, which includes `CheckAlways` views.
+      // If the root view has been explicitly marked for check, we also need Global mode.
+      ChangeDetectionMode.Global :
+      // Only refresh views with the `RefreshView` flag or views is a changed signal
+      ChangeDetectionMode.Targeted;
   detectChangesInternal(lView, notifyErrorHandler, mode);
 }

packages/core/src/render3/instructions/change_detection.ts

@@ -107,7 +107,7 @@ export const enum ChangeDetectionMode {
    */
   Global,
   /**
-   * In `Targeted` mode, only views with the `RefreshView` flag are refreshed.
+   * In `Targeted` mode, only views with the `RefreshView` flag or updated signals are refreshed.
    */
   Targeted,
 }

packages/core/test/application_ref_spec.ts

@@ -8,7 +8,7 @@
 
 import {DOCUMENT, ɵgetDOM as getDOM} from '@angular/common';
 import {ResourceLoader} from '@angular/compiler';
-import {APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, Compiler, CompilerFactory, Component, EnvironmentInjector, InjectionToken, LOCALE_ID, NgModule, NgZone, PlatformRef, RendererFactory2, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core';
+import {APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, ChangeDetectionStrategy, Compiler, CompilerFactory, Component, EnvironmentInjector, InjectionToken, LOCALE_ID, NgModule, NgZone, PlatformRef, RendererFactory2, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core';
 import {ErrorHandler} from '@angular/core/src/error_handler';
 import {ComponentRef} from '@angular/core/src/linker/component_factory';
 import {createEnvironmentInjector, getLocaleId} from '@angular/core/src/render3';
@@ -632,6 +632,32 @@ describe('bootstrap', () => {
       expect(comp.nativeElement).toHaveText('Initial');
     });
 
+    it('should not dirty host bindings of views not marked for check', () => {
+      @Component({
+        template: '',
+        host: {'[class]': 'clazz'},
+        standalone: true,
+        changeDetection: ChangeDetectionStrategy.OnPush
+      })
+      class HostBindingComp {
+        clazz = 'initial';
+      }
+      const comp = TestBed.createComponent(HostBindingComp);
+      const appRef = TestBed.inject(ApplicationRef);
+
+      appRef.attachView(comp.componentRef.hostView);
+      appRef.tick();
+      expect(comp.nativeElement.outerHTML).toContain('initial');
+
+      comp.componentInstance.clazz = 'new';
+      appRef.tick();
+      expect(comp.nativeElement.outerHTML).toContain('initial');
+
+      comp.changeDetectorRef.markForCheck();
+      appRef.tick();
+      expect(comp.nativeElement.outerHTML).toContain('new');
+    });
+
     it('should detach attached views if they are destroyed', () => {
       const comp = TestBed.createComponent(MyComp);
       const appRef: ApplicationRef = TestBed.inject(ApplicationRef);

packages/core/testing/src/component_fixture.ts

@@ -328,11 +328,19 @@ export class PseudoApplicationComponentFixture<T> extends ComponentFixture<T> {
       });
       this.beforeRenderSubscription = this._testAppRef.beforeRender.subscribe((isFirstPass) => {
         try {
-          ɵdetectChangesInViewIfRequired(
-              (this.componentRef.hostView as any)._lView,
-              isFirstPass,
-              (this.componentRef.hostView as any).notifyErrorHandler,
-          );
+          if (isFirstPass) {
+            // TODO(atscott): This matches old behavior where change detection was forced on every
+            // microtask empty, ignoring OnPush. This is incorrect and should be fixed in a major
+            // version if possible.
+            this.changeDetectorRef.detectChanges();
+          } else {
+            ɵdetectChangesInViewIfRequired(
+                (this.componentRef.hostView as any)._lView,
+                isFirstPass,
+                (this.componentRef.hostView as any).notifyErrorHandler,
+            );
+          }
+
         } catch (e: unknown) {
           // If an error ocurred during change detection, remove the test view from the application
           // ref tracking. Note that this isn't exactly desirable but done this way because of how