From df292c2ce0805de4087fae0afd26277c221bc9c4 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 9 Jan 2019 16:24:36 -0800 Subject: [PATCH] fix(ivy): TestBed should not clobber compilation of global-scope modules (#28033) When an @NgModule decorator executes, the module is added to a queue in render3/jit/module.ts. Reading an ngComponentDef property causes this queue to be flushed, ensuring that the component gets the correct module scope applied. In before_each.ts, a global beforeEach is added to all Angular tests which calls TestBed.resetTestingModule() prior to running each test. This in turn clears the module compilation queue (which is correct behavior, as modules declared within the test should not leak outside of it via the queue). So far this is okay. But before the first test runs, the module compilation queue is full of modules declared in global scope. No definitions have been read, so no flushes of the queue have been triggered. The global beforeEach triggers a reset of the queue, aborting all of the in-progress global compilation, breaking those classes when they're later used in tests. This commit adds logic to TestBedRender3 to respect the state of the module queue before the TestBed is first initialized or reset. The queue is flushed prior to such an operation to ensure global compilation is allowed to finish properly. With this fix, a platform-server test now passes (previously the element was not detected as a component, because the encompassing module never finished compilation. FW-887 #resolve PR Close #28033 --- .../core/src/core_render3_private_export.ts | 1 + packages/core/testing/src/r3_test_bed.ts | 25 +++++++++++++++++++ .../platform-server/test/integration_spec.ts | 17 ++++++------- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/packages/core/src/core_render3_private_export.ts b/packages/core/src/core_render3_private_export.ts index c80ad949c9..e0a8e77a03 100644 --- a/packages/core/src/core_render3_private_export.ts +++ b/packages/core/src/core_render3_private_export.ts @@ -135,6 +135,7 @@ export { compileNgModuleDefs as ɵcompileNgModuleDefs, patchComponentDefWithScope as ɵpatchComponentDefWithScope, resetCompiledComponents as ɵresetCompiledComponents, + flushModuleScopingQueueAsMuchAsPossible as ɵflushModuleScopingQueueAsMuchAsPossible, transitiveScopesFor as ɵtransitiveScopesFor, } from './render3/jit/module'; export { diff --git a/packages/core/testing/src/r3_test_bed.ts b/packages/core/testing/src/r3_test_bed.ts index 6e558f7853..debfc9e285 100644 --- a/packages/core/testing/src/r3_test_bed.ts +++ b/packages/core/testing/src/r3_test_bed.ts @@ -43,6 +43,7 @@ import { ɵcompileNgModuleDefs as compileNgModuleDefs, ɵcompilePipe as compilePipe, ɵgetInjectableDef as getInjectableDef, + ɵflushModuleScopingQueueAsMuchAsPossible as flushModuleScopingQueueAsMuchAsPossible, ɵpatchComponentDefWithScope as patchComponentDefWithScope, ɵresetCompiledComponents as resetCompiledComponents, ɵstringify as stringify, ɵtransitiveScopesFor as transitiveScopesFor, @@ -245,6 +246,7 @@ export class TestBedRender3 implements Injector, TestBed { private _testModuleType: NgModuleType = null !; private _instantiated: boolean = false; + private _globalCompilationChecked = false; // Map that keeps initial version of component/directive/pipe defs in case // we compile a Type again, thus overriding respective static fields. This is @@ -285,6 +287,7 @@ export class TestBedRender3 implements Injector, TestBed { } resetTestingModule(): void { + this._checkGlobalCompilationFinished(); resetCompiledComponents(); // reset metadata overrides this._moduleOverrides = []; @@ -459,6 +462,7 @@ export class TestBedRender3 implements Injector, TestBed { // internal methods private _initIfNeeded(): void { + this._checkGlobalCompilationFinished(); if (this._instantiated) { return; } @@ -626,6 +630,27 @@ export class TestBedRender3 implements Injector, TestBed { patchComponentDefWithScope((cmp as any).ngComponentDef, scope); }); } + + /** + * Check whether the module scoping queue should be flushed, and flush it if needed. + * + * When the TestBed is reset, it clears the JIT module compilation queue, cancelling any + * in-progress module compilation. This creates a potential hazard - the very first time the + * TestBed is initialized (or if it's reset without being initialized), there may be pending + * compilations of modules declared in global scope. These compilations should be finished. + * + * To ensure that globally declared modules have their components scoped properly, this function + * is called whenever TestBed is initialized or reset. The _first_ time that this happens, prior + * to any other operations, the scoping queue is flushed. + */ + private _checkGlobalCompilationFinished(): void { + // !this._instantiated should not be necessary, but is left in as an additional guard that + // compilations queued in tests (after instantiation) are never flushed accidentally. + if (!this._globalCompilationChecked && !this._instantiated) { + flushModuleScopingQueueAsMuchAsPossible(); + } + this._globalCompilationChecked = true; + } } let testBed: TestBedRender3; diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index c15e0256a9..8a4a25d2c2 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -617,15 +617,14 @@ class HiddenModule { }); })); - fixmeIvy('FW-887: JIT: compilation of NgModule') - .it('should handle false values on attributes', async(() => { - renderModule(FalseAttributesModule, {document: doc}).then(output => { - expect(output).toBe( - '' + - 'Works!'); - called = true; - }); - })); + it('should handle false values on attributes', async(() => { + renderModule(FalseAttributesModule, {document: doc}).then(output => { + expect(output).toBe( + '' + + 'Works!'); + called = true; + }); + })); it('should handle element property "name"', async(() => { renderModule(NameModule, {document: doc}).then(output => {