TestBed should be zoneless compatible

[yjaaidi]

Which @angular/* package(s) are relevant/related to the feature request?

core

Description

TL;DR: TestBed should provide a way to run zoneless and avoid importing zone.js when we don't need it.
And I'd love to help with a PR?

TestBed assumes that zone.js is available while there are contexts when this is not necessary and here are some examples:

• zoneless apps & libs
• tests that do not depend on change detection: service tests, pipe tests, "isolated" component testing etc...
• tests where CD is triggered manually using fixture.detectChanges()

These tests must load zone.js when they don't need it.
While this might sound like a really minor issue, loading zone.js can cause trouble in some situations.

For instance, integrating TestBed and zone.js with vitest causes the following error Method Promise.prototype.then called on incompatible receiver which is probably due to zone.js patching promises and breaking Node.js's safe promises when importing ESMs dynamically or something like that. This is similar to the issue described here #47872.

Proposed solution

A first quick win is replacing this:

angular/packages/core/testing/src/test_bed_compiler.ts

Lines 749 to 751 in 6948f0f

	const ngZone = new NgZone({enableLongStackTrace: true});
	const providers: Provider[] = [
	{provide: NgZone, useValue: ngZone},

with:

{provide: NgZone, useFactory: () => new NgZone({enableLongStackTrace: true}); }

this would then allow us to go zoneless with this configuration:

TestBed.configureTestingModule({
  providers: [
    {
      provide: NgZone,
      useClass: ɵNoopNgZone,
    },
  ],
});

The main problem with this approach is the boilerplate and having to use the ɵNoopNgZone implementation detail.

A second step would be adding the bootstrap's ngZone option to initTestEnvironment:

getTestBed().initTestEnvironment(
  BrowserDynamicTestingModule,
  platformBrowserDynamicTesting(),
  {ngZone: 'noop'}
);

Alternatives considered

Disable promises monkey patching using globalThis.__Zone_disable_ZoneAwarePromise = true
but this causes the following issue #37349
Cf. #48359

[AndrewKushnir]

@yjaaidi thanks for submitting this ticket.

The first step of making the new NgZone creation lazy makes sense, please let us know if you are interested in submitting a PR. As you mentioned, that should provide an ability to replace ZoneJS implementation and experiment with vitest integration.

The second step of exposing a config option would require some discussions within the team. I'll share an update once I have it.

Thank you.

[yjaaidi]

Thanks @AndrewKushnir for your quick feedback.
This is great! And I'll be happy to help a PR pretty soon 😉

While investigating a bit further, I discovered another spot.
When there is a global afterEach (jasmine, jest, cypress, vitest with globals set to true), TestBed's cleanup hook calls resetFakeAsyncZone which throws the following error.

angular/packages/core/testing/src/fake_async.ts

Line 25 in 0ff5d97

throw new Error(fakeAsyncTestModuleNotLoadedErrorMessage);

IMO, this shouldn't throw an error. The error doesn't make much sense if we're not using fakeAsync and fakeAsync already throws the error.

Then the function should actually be named something like tryResetFakeAsyncZone but as the function is part of the public API we will have to keep it as is or alias it resetFakeAsyncZone = tryResetFakeAsyncZone.

[AndrewKushnir]

@yjaaidi the mentioned call is likely coming from here:

angular/packages/core/testing/src/test_hooks.ts

Lines 34 to 42 in 4d398a0

	function getCleanupHook(expectedTeardownValue: boolean) {
	return () => {
	const testBed = TestBedImpl.INSTANCE;
	if (testBed.shouldTearDownTestingModule() === expectedTeardownValue) {
	testBed.resetTestingModule();
	resetFakeAsyncZone();
	}
	};
	}

We need to experiment with this more, but we can probably update it eventually to detect if we are in "zone-less" mode:

function getCleanupHook(expectedTeardownValue: boolean) {
  return () => {
    const testBed = TestBedImpl.INSTANCE;
    if (testBed.shouldTearDownTestingModule() === expectedTeardownValue) {
      const ngZone = testBed.get(NgZone); // access injector before it gets destroyed in the call below
      testBed.resetTestingModule();
      if (!(ngZone instanceof ɵNoopNgZone)) {
        resetFakeAsyncZone();
      }
    }
  };
}

[yjaaidi]

Thanks @AndrewKushnir ! In fact, this could be a quick win.
Though, I still think that throwing an error about fakeAsync while nobody is using it is misleading. What do you think?

If someone is using fakeAsync, the error is already thrown anyway.

[AndrewKushnir]

If someone is using fakeAsync, the error is already thrown anyway.

@yjaaidi could you please let me know which error you refer to?

[yjaaidi]

@AndrewKushnir it's this one:

angular/packages/core/testing/src/fake_async.ts

Lines 51 to 56 in 0ff5d97

	export function fakeAsync(fn: Function): (...args: any[]) => any {
	if (fakeAsyncTestModule) {
	return fakeAsyncTestModule.fakeAsync(fn);
	}
	throw new Error(fakeAsyncTestModuleNotLoadedErrorMessage);
	}

[LayZeeDK]

@yjaaidi

In component tests, the NgZone is disabled by providing true as the value of ComponentFixtureNoNgZone as seen in

angular/packages/core/test/application_ref_spec.ts

Lines 610 to 613 in 4e098fa

	TestBed.configureTestingModule({
	declarations: [MyComp, ContainerComp, EmbeddedViewComp],
	providers: [{provide: ComponentFixtureNoNgZone, useValue: true}]
	});

Example

import { ComponentFixtureNoNgZone, TestBed } from '@angular/core/testing'
import { MyComponent } from './my.component';

it('supports zoneless', () => {
  TestBed.configureTestingModule({
    providers: [
      { provide: ComponentFixtureNoNgZone, useValue: true },
    ],
  });
  
  const fixture = TestBed.createComponent(MyComponent);
});

The tests for ComponentFixtureNoNgZone are at

angular/packages/core/test/component_fixture_spec.ts

Lines 298 to 329 in 8c04fa7

	describe('No NgZone', () => {
	beforeEach(() => {
	TestBed.configureTestingModule(
	{providers: [{provide: ComponentFixtureNoNgZone, useValue: true}]});
	});
	it('calling autoDetectChanges raises an error', () => {
	const componentFixture = TestBed.createComponent(SimpleComp);
	expect(() => {
	componentFixture.autoDetectChanges();
	}).toThrowError(/Cannot call autoDetectChanges when ComponentFixtureNoNgZone is set/);
	});
	it('should instantiate a component with valid DOM', waitForAsync(() => {
	const componentFixture = TestBed.createComponent(SimpleComp);
	expect(componentFixture.ngZone).toBeNull();
	componentFixture.detectChanges();
	expect(componentFixture.nativeElement).toHaveText('Original Simple');
	}));
	it('should allow changing members of the component', waitForAsync(() => {
	const componentFixture = TestBed.createComponent(MyIfComp);
	componentFixture.detectChanges();
	expect(componentFixture.nativeElement).toHaveText('MyIf()');
	componentFixture.componentInstance.showMore = true;
	componentFixture.detectChanges();
	expect(componentFixture.nativeElement).toHaveText('MyIf(More)');
	}));
	});

The all but missing docs for ComponentFixtureNoNgZone are at https://angular.io/api/core/testing/ComponentFixtureNoNgZone

[yjaaidi]

Hey @LayZeeDK !

Thanks for the help! Actually, ComponentFixtureNoNgZone skips injecting NgZone but NgZone was still instantiated and provided here (even if it's never injected nor used).

angular/packages/core/testing/src/test_bed_compiler.ts

Lines 749 to 751 in 6948f0f

	const ngZone = new NgZone({enableLongStackTrace: true});
	const providers: Provider[] = [
	{provide: NgZone, useValue: ngZone},

But then it's already too late as the constructor of NgZone asserts the presence of Zone.js:

angular/packages/core/src/zone/ng_zone.ts

Lines 128 to 130 in 5e404d3

	if (typeof Zone == 'undefined') {
	throw new Error(`In this configuration Angular requires Zone.js`);
	}

This made it mandatory to import zone.js even if it's not used... and that's what doesn't play well with Vitest & ESM in Node (due to promise monkey patching).

But guess what! Thanks to your comment, I checked again and this was indirectly fixed by the new provideNgZoneChangeDetection() function and mainly this commit d7d6514 by @atscott

Thanks so much to both of you!!!

Also thank you @AndrewKushnir ! and sorry for the delay... I was off for some time and still catching up.

[yjaaidi]

This was partially and indirectly solved by #49557 by @atscott

Now, we still have to fix the clean up: #48198 (comment)

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}