Skip to content

Commit

Permalink
fix(core): ComponentFixture autoDetect feature works like production
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
atscott committed Aug 12, 2024
1 parent 52673e6 commit 7419fa1
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 54 deletions.
6 changes: 3 additions & 3 deletions packages/core/src/application/application_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
42 changes: 42 additions & 0 deletions packages/core/test/component_fixture_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
ErrorHandler,
Injectable,
Input,
NgZone,
createComponent,
provideExperimentalZonelessChangeDetection,
signal,
Expand Down Expand Up @@ -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', () => {
Expand Down
78 changes: 28 additions & 50 deletions packages/core/testing/src/component_fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
ApplicationRef,
ChangeDetectorRef,
ComponentRef,
ɵChangeDetectionScheduler,
ɵNotificationSource,
DebugElement,
ElementRef,
getDebugNode,
Expand All @@ -18,7 +20,6 @@ import {
RendererFactory2,
ViewRef,
ɵDeferBlockDetails as DeferBlockDetails,
ɵdetectChangesInViewIfRequired,
ɵEffectScheduler as EffectScheduler,
ɵgetDeferBlocks as getDeferBlocks,
ɵNoopNgZone as NoopNgZone,
Expand Down Expand Up @@ -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;
Expand All @@ -95,6 +99,16 @@ export abstract class ComponentFixture<T> {
this.componentRef = componentRef;
}

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.
*/
Expand Down Expand Up @@ -180,6 +194,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;
Expand All @@ -194,9 +209,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);
}
Expand Down Expand Up @@ -230,26 +247,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(() => {
Expand Down Expand Up @@ -285,54 +300,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();
}
Expand Down

0 comments on commit 7419fa1

Please sign in to comment.