From 0f23f7343ec9bdcd2d40f6f474d2a4f5e38e2627 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sat, 26 Jun 2021 11:28:12 +0200 Subject: [PATCH] fix(core): error in TestBed if module is reset mid-compilation in ViewEngine (#42669) When `TestBed.compileComponents` is called under ViewEngine, we kick off a compilation and return a promise that resolves once the compilation is done. In most cases the consumer doesn't _have_ to await the returned promise, unless their components have external resources. The problem is that the test could be over by the time the promise has resolved, in which case we still cache the factory of the test module. This becomes a problem if another compilation is triggered right afterwards, because it'll see that we still have a `_moduleFactory` and it won't recreate the factory. These changes resolve the issue by saving a reference to the module type that is being compiled and checking against it when the promise resolves. Note that while this problem was discovered while trying to roll out the new test module teardown behavior in the Components repo (https://github.com/angular/components/pull/23070), it has been there for a long time. The new test behavior made it more apparent. PR Close #42669 --- packages/core/test/test_bed_spec.ts | 29 +++++++++++++++++++++++++++ packages/core/testing/src/test_bed.ts | 20 ++++++++++++------ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 6d360229a0..5874e4e888 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -307,6 +307,35 @@ describe('TestBed', () => { expect(SimpleService.ngOnDestroyCalls).toBe(0); }); + it('should be able to create a fixture if a test module is reset mid-compilation', async () => { + const token = new InjectionToken('value'); + + @Component({template: 'hello {{_token}}'}) + class TestComponent { + constructor(@Inject(token) public _token: number) {} + } + + TestBed.resetTestingModule(); // Reset the state from `beforeEach`. + + function compile(tokenValue: number) { + return TestBed + .configureTestingModule({ + declarations: [TestComponent], + providers: [{provide: token, useValue: tokenValue}], + teardown: {destroyAfterEach: true} + }) + .compileComponents(); + } + + const initialCompilation = compile(1); + TestBed.resetTestingModule(); + await initialCompilation; + await compile(2); + const fixture = TestBed.createComponent(TestComponent); + fixture.detectChanges(); + expect(fixture.nativeElement).toHaveText('hello 2'); + }); + describe('module overrides using TestBed.overrideModule', () => { @Component({ selector: 'test-cmp', diff --git a/packages/core/testing/src/test_bed.ts b/packages/core/testing/src/test_bed.ts index 627489c3e0..61c4a05357 100644 --- a/packages/core/testing/src/test_bed.ts +++ b/packages/core/testing/src/test_bed.ts @@ -263,7 +263,8 @@ export class TestBedViewEngine implements TestBed { private _compiler: TestingCompiler = null!; private _moduleRef: NgModuleRef|null = null; - private _moduleFactory: NgModuleFactory = null!; + private _moduleFactory: NgModuleFactory|null = null; + private _pendingModuleFactory: Type|null = null; private _compilerOptions: CompilerOptions[] = []; @@ -341,7 +342,8 @@ export class TestBedViewEngine implements TestBed { this._isRoot = true; this._rootProviderOverrides = []; - this._moduleFactory = null!; + this._moduleFactory = null; + this._pendingModuleFactory = null; this._compilerOptions = []; this._providers = []; this._declarations = []; @@ -399,10 +401,16 @@ export class TestBedViewEngine implements TestBed { } const moduleType = this._createCompilerAndModule(); - return this._compiler.compileModuleAndAllComponentsAsync(moduleType) - .then((moduleAndComponentFactories) => { - this._moduleFactory = moduleAndComponentFactories.ngModuleFactory; - }); + this._pendingModuleFactory = moduleType; + return this._compiler.compileModuleAndAllComponentsAsync(moduleType).then(result => { + // If the module mismatches by the time the promise resolves, it means that the module has + // already been destroyed and a new compilation has started. If that's the case, avoid + // overwriting the module factory, because it can cause downstream errors. + if (this._pendingModuleFactory === moduleType) { + this._moduleFactory = result.ngModuleFactory; + this._pendingModuleFactory = null; + } + }); } private _initIfNeeded(): void {