fix(core): ComponentFixture autoDetect feature works like production (#55228)

atscott · dylhunn · commit f03d274e87c9 · 2024-08-15T15:45:45.000-04:00
This commit fully integrates the `autoDetect` feature into `ApplicationRef.tick` without special handling for errors. This commit also shares the method of autoDetect for change detection between the zoneless and zone component fixture implementations. The difference is now limited to: * autoDetect is defaulted to true with zoneless * detectChanges with zoneless is AppRef.tick while it is ChangeDetectorRef.detectChanges with zones. This should likely converge more in the future. Not going through AppRef.tick means that the zone fixture does not get guaranteed `afterRender` executions and does not get the rerunning behavior if the fixture is marked dirty by a render hook. BREAKING CHANGE: The `autoDetect` feature of `ComponentFixture` will now attach the fixture to the `ApplicationRef`. As a result, errors during automatic change detection of the fixture be reported to the `ErrorHandler`. This change may cause custom error handlers to observe new failures that were previously unreported. PR Close #55228
diff --git a/packages/core/src/application/application_ref.ts b/packages/core/src/application/application_ref.ts
@@ -318,7 +318,7 @@ export class ApplicationRef {
   /** @internal */
   afterTick = new Subject<void>();
   /** @internal */
-  get allViews() {
+  get allViews(): Array<InternalViewRef<unknown>> {
     return [...this.externalTestViews.keys(), ...this._views];
   }
 
@@ -577,7 +577,7 @@ export class ApplicationRef {
       this.detectChangesInAttachedViews(refreshViews);
 
       if (typeof ngDevMode === 'undefined' || ngDevMode) {
-        for (let view of this._views) {
+        for (let view of this.allViews) {
           view.checkNoChanges();
         }
       }
@@ -605,7 +605,7 @@ export class ApplicationRef {
       // After the we execute render hooks in the first pass, we loop while views are marked dirty and should refresh them.
       if (refreshViews || !isFirstPass) {
         this.beforeRender.next(isFirstPass);
-        for (let {_lView, notifyErrorHandler} of this._views) {
+        for (let {_lView, notifyErrorHandler} of this.allViews) {
           detectChangesInViewIfRequired(
             _lView,
             notifyErrorHandler,
diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts
@@ -210,7 +210,8 @@ export function createLView<T>(
     LViewFlags.CreationMode |
     LViewFlags.Attached |
     LViewFlags.FirstLViewPass |
-    LViewFlags.Dirty;
+    LViewFlags.Dirty |
+    LViewFlags.RefreshView;
   if (
     embeddedViewInjector !== null ||
     (parentLView && parentLView[FLAGS] & LViewFlags.HasEmbeddedViewInjector)
diff --git a/packages/core/test/component_fixture_spec.ts b/packages/core/test/component_fixture_spec.ts
@@ -13,6 +13,7 @@ import {
   ErrorHandler,
   Injectable,
   Input,
+  NgZone,
   createComponent,
   provideExperimentalZonelessChangeDetection,
   signal,
@@ -460,6 +461,47 @@ describe('ComponentFixture', () => {
       expect(() => fixture.detectChanges()).toThrow();
     });
   });
+
+  it('reports errors from autoDetect change detection to error handler', () => {
+    let throwError = false;
+    @Component({template: ''})
+    class TestComponent {
+      ngDoCheck() {
+        if (throwError) {
+          throw new Error();
+        }
+      }
+    }
+    const fixture = TestBed.createComponent(TestComponent);
+    fixture.autoDetectChanges();
+    const errorHandler = TestBed.inject(ErrorHandler);
+    const spy = spyOn(errorHandler, 'handleError').and.callThrough();
+
+    throwError = true;
+    TestBed.inject(NgZone).run(() => {});
+    expect(spy).toHaveBeenCalled();
+  });
+
+  it('reports errors from checkNoChanges in autoDetect to error handler', () => {
+    let throwError = false;
+    @Component({template: '{{thing}}'})
+    class TestComponent {
+      thing = 'initial';
+      ngAfterViewChecked() {
+        if (throwError) {
+          this.thing = 'new';
+        }
+      }
+    }
+    const fixture = TestBed.createComponent(TestComponent);
+    fixture.autoDetectChanges();
+    const errorHandler = TestBed.inject(ErrorHandler);
+    const spy = spyOn(errorHandler, 'handleError').and.callThrough();
+
+    throwError = true;
+    TestBed.inject(NgZone).run(() => {});
+    expect(spy).toHaveBeenCalled();
+  });
 });
 
 describe('ComponentFixture with zoneless', () => {
diff --git a/packages/core/testing/src/component_fixture.ts b/packages/core/testing/src/component_fixture.ts
@@ -10,6 +10,8 @@ import {
   ApplicationRef,
   ChangeDetectorRef,
   ComponentRef,
+  ɵChangeDetectionScheduler,
+  ɵNotificationSource,
   DebugElement,
   ElementRef,
   getDebugNode,
@@ -18,7 +20,6 @@ import {
   RendererFactory2,
   ViewRef,
   ɵDeferBlockDetails as DeferBlockDetails,
-  ɵdetectChangesInViewIfRequired,
   ɵEffectScheduler as EffectScheduler,
   ɵgetDeferBlocks as getDeferBlocks,
   ɵNoopNgZone as NoopNgZone,
@@ -81,6 +82,9 @@ export abstract class ComponentFixture<T> {
   protected readonly _testAppRef = this._appRef as unknown as TestAppRef;
   private readonly pendingTasks = inject(PendingTasks);
   private readonly appErrorHandler = inject(TestBedApplicationErrorHandler);
+  /** @internal */
+  protected abstract _autoDetect: boolean;
+  private readonly scheduler = inject(ɵChangeDetectionScheduler, {optional: true});
 
   // TODO(atscott): Remove this from public API
   ngZone = this._noZoneOptionIsSet ? null : this._ngZone;
@@ -95,6 +99,17 @@ export abstract class ComponentFixture<T> {
     this.componentRef = componentRef;
   }
 
+  /** @internal */
+  initialize(): void {
+    if (this._autoDetect) {
+      this._testAppRef.externalTestViews.add(this.componentRef.hostView);
+      this.scheduler?.notify(ɵNotificationSource.ViewAttached);
+    }
+    this.componentRef.hostView.onDestroy(() => {
+      this._testAppRef.externalTestViews.delete(this.componentRef.hostView);
+    });
+  }
+
   /**
    * Trigger a change detection cycle for the component.
    */
@@ -180,6 +195,7 @@ export abstract class ComponentFixture<T> {
    * Trigger component destruction.
    */
   destroy(): void {
+    this._testAppRef.externalTestViews.delete(this.componentRef.hostView);
     if (!this._isDestroyed) {
       this.componentRef.destroy();
       this._isDestroyed = true;
@@ -194,9 +210,11 @@ export abstract class ComponentFixture<T> {
  * `ApplicationRef.isStable`, and `autoDetectChanges` cannot be disabled.
  */
 export class ScheduledComponentFixture<T> extends ComponentFixture<T> {
-  private _autoDetect = inject(ComponentFixtureAutoDetect, {optional: true}) ?? true;
+  /** @internal */
+  protected override _autoDetect = inject(ComponentFixtureAutoDetect, {optional: true}) ?? true;
 
-  initialize(): void {
+  override initialize(): void {
+    super.initialize();
     if (this._autoDetect) {
       this._appRef.attachView(this.componentRef.hostView);
     }
@@ -230,26 +248,24 @@ export class ScheduledComponentFixture<T> extends ComponentFixture<T> {
 
 interface TestAppRef {
   externalTestViews: Set<ViewRef>;
-  beforeRender: Subject<boolean>;
-  afterTick: Subject<void>;
 }
 
 /**
  * ComponentFixture behavior that attempts to act as a "mini application".
  */
 export class PseudoApplicationComponentFixture<T> extends ComponentFixture<T> {
   private _subscriptions = new Subscription();
-  private _autoDetect = inject(ComponentFixtureAutoDetect, {optional: true}) ?? false;
-  private afterTickSubscription: Subscription | undefined = undefined;
-  private beforeRenderSubscription: Subscription | undefined = undefined;
+  /** @internal */
+  override _autoDetect = inject(ComponentFixtureAutoDetect, {optional: true}) ?? false;
 
-  initialize(): void {
+  override initialize(): void {
     if (this._autoDetect) {
-      this.subscribeToAppRefEvents();
+      this._testAppRef.externalTestViews.add(this.componentRef.hostView);
     }
     this.componentRef.hostView.onDestroy(() => {
-      this.unsubscribeFromAppRefEvents();
+      this._testAppRef.externalTestViews.delete(this.componentRef.hostView);
     });
+
     // Create subscriptions outside the NgZone so that the callbacks run outside
     // of NgZone.
     this._ngZone.runOutsideAngular(() => {
@@ -285,54 +301,17 @@ export class PseudoApplicationComponentFixture<T> extends ComponentFixture<T> {
 
     if (autoDetect !== this._autoDetect) {
       if (autoDetect) {
-        this.subscribeToAppRefEvents();
+        this._testAppRef.externalTestViews.add(this.componentRef.hostView);
       } else {
-        this.unsubscribeFromAppRefEvents();
+        this._testAppRef.externalTestViews.delete(this.componentRef.hostView);
       }
     }
 
     this._autoDetect = autoDetect;
     this.detectChanges();
   }
 
-  private subscribeToAppRefEvents() {
-    this._ngZone.runOutsideAngular(() => {
-      this.afterTickSubscription = this._testAppRef.afterTick.subscribe(() => {
-        this.checkNoChanges();
-      });
-      this.beforeRenderSubscription = this._testAppRef.beforeRender.subscribe((isFirstPass) => {
-        try {
-          ɵdetectChangesInViewIfRequired(
-            (this.componentRef.hostView as any)._lView,
-            (this.componentRef.hostView as any).notifyErrorHandler,
-            isFirstPass,
-            false /** zoneless enabled */,
-          );
-        } catch (e: unknown) {
-          // If an error occurred 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
-          // things used to work with `autoDetect` and uncaught errors. Ideally we would surface
-          // this error to the error handler instead and continue refreshing the view like
-          // what would happen in the application.
-          this.unsubscribeFromAppRefEvents();
-
-          throw e;
-        }
-      });
-      this._testAppRef.externalTestViews.add(this.componentRef.hostView);
-    });
-  }
-
-  private unsubscribeFromAppRefEvents() {
-    this.afterTickSubscription?.unsubscribe();
-    this.beforeRenderSubscription?.unsubscribe();
-    this.afterTickSubscription = undefined;
-    this.beforeRenderSubscription = undefined;
-    this._testAppRef.externalTestViews.delete(this.componentRef.hostView);
-  }
-
   override destroy(): void {
-    this.unsubscribeFromAppRefEvents();
     this._subscriptions.unsubscribe();
     super.destroy();
   }
