From 3deda898d0c0c58ac07c7ca4c0196788ef007ce5 Mon Sep 17 00:00:00 2001
From: Jeremy Elbourn <jelbourn@google.com>
Date: Wed, 23 Jan 2019 15:09:27 -0800
Subject: [PATCH] fix(ivy): TestBed should tolerate synchronous use of
 `compileComponents` (#28350)

TestBed.compileComponents has always been an async API. However,
ViewEngine tolerated using this API in a synchronous manner if the
components declared in the testing module did not have any async
resources (templateUrl, styleUrls). This change makes the ivy TestBed
mirror this tolerance by configuring such components synchronously.

Ref: FW-992

PR Close #28350
---
 .../core/src/metadata/resource_loading.ts     |  6 +--
 packages/core/test/test_bed_spec.ts           | 24 +++++++++++
 packages/core/testing/src/r3_test_bed.ts      | 43 ++++++++++++-------
 3 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/packages/core/src/metadata/resource_loading.ts b/packages/core/src/metadata/resource_loading.ts
index 257c937a27..26691ce10a 100644
--- a/packages/core/src/metadata/resource_loading.ts
+++ b/packages/core/src/metadata/resource_loading.ts
@@ -91,8 +91,8 @@ export function maybeQueueResolutionOfComponentResources(metadata: Component) {
   }
 }
 
-export function componentNeedsResolution(component: Component) {
-  return component.templateUrl || component.styleUrls && component.styleUrls.length;
+export function componentNeedsResolution(component: Component): boolean {
+  return !!(component.templateUrl || component.styleUrls && component.styleUrls.length);
 }
 export function clearResolutionOfComponentResourcesQueue() {
   componentResourceResolutionQueue.clear();
@@ -100,4 +100,4 @@ export function clearResolutionOfComponentResourcesQueue() {
 
 function unwrapResponse(response: string | {text(): Promise<string>}): string|Promise<string> {
   return typeof response == 'string' ? response : response.text();
-}
\ No newline at end of file
+}
diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts
index c76adf23e8..d1a0d2f3ae 100644
--- a/packages/core/test/test_bed_spec.ts
+++ b/packages/core/test/test_bed_spec.ts
@@ -79,6 +79,10 @@ export class ComponentWithPropBindings {
 export class SimpleApp {
 }
 
+@Component({selector: 'inline-template', template: '<p>Hello</p>'})
+export class ComponentWithInlineTemplate {
+}
+
 @NgModule({
   declarations: [
     HelloWorld, SimpleCmp, WithRefsCmp, InheritedCmp, SimpleApp, ComponentWithPropBindings,
@@ -232,6 +236,26 @@ describe('TestBed', () => {
     expect(simpleApp.nativeElement).toHaveText('simple - inherited');
   });
 
+  it('should resolve components without async resources synchronously', (done) => {
+    TestBed
+        .configureTestingModule({
+          declarations: [ComponentWithInlineTemplate],
+        })
+        .compileComponents()
+        .then(done)
+        .catch(error => {
+          // This should not throw any errors. If an error is thrown, the test will fail.
+          // Specifically use `catch` here to mark the test as done and *then* throw the error
+          // so that the test isn't treated as a timeout.
+          done();
+          throw error;
+        });
+
+    // Intentionally call `createComponent` before `compileComponents` is resolved. We want this to
+    // work for components that don't have any async resources (templateUrl, styleUrls).
+    TestBed.createComponent(ComponentWithInlineTemplate);
+  });
+
   onlyInIvy('patched ng defs should be removed after resetting TestingModule')
       .it('make sure we restore ng defs to their initial states', () => {
         @Pipe({name: 'somePipe', pure: true})
diff --git a/packages/core/testing/src/r3_test_bed.ts b/packages/core/testing/src/r3_test_bed.ts
index 5eec6ba78e..442e7b8ff1 100644
--- a/packages/core/testing/src/r3_test_bed.ts
+++ b/packages/core/testing/src/r3_test_bed.ts
@@ -54,7 +54,7 @@ import {
 // clang-format on
 import {ResourceLoader} from '@angular/compiler';
 
-import {clearResolutionOfComponentResourcesQueue, resolveComponentResources} from '../../src/metadata/resource_loading';
+import {clearResolutionOfComponentResourcesQueue, componentNeedsResolution, resolveComponentResources} from '../../src/metadata/resource_loading';
 import {ComponentFixture} from './component_fixture';
 import {MetadataOverride} from './metadata_override';
 import {ComponentResolver, DirectiveResolver, NgModuleResolver, PipeResolver, Resolver} from './resolvers';
@@ -374,6 +374,8 @@ export class TestBedRender3 implements Injector, TestBed {
     const declarations: Type<any>[] = flatten(this._declarations || EMPTY_ARRAY, resolveForwardRef);
 
     const componentOverrides: [Type<any>, Component][] = [];
+    let hasAsyncResources = false;
+
     // Compile the components declared by this module
     declarations.forEach(declaration => {
       const component = resolvers.component.resolve(declaration);
@@ -382,25 +384,34 @@ export class TestBedRender3 implements Injector, TestBed {
         const metadata = {...component};
         compileComponent(declaration, metadata);
         componentOverrides.push([declaration, metadata]);
+        hasAsyncResources = hasAsyncResources || componentNeedsResolution(component);
       }
     });
 
-    let resourceLoader: ResourceLoader;
+    const overrideComponents = () => {
+      componentOverrides.forEach((override: [Type<any>, Component]) => {
+        // Override the existing metadata, ensuring that the resolved resources
+        // are only available until the next TestBed reset (when `resetTestingModule` is called)
+        this.overrideComponent(override[0], {set: override[1]});
+      });
+    };
 
-    return resolveComponentResources(url => {
-             if (!resourceLoader) {
-               resourceLoader = this.compilerInjector.get(ResourceLoader);
-             }
-             return Promise.resolve(resourceLoader.get(url));
-           })
-        .then(() => {
-          componentOverrides.forEach((override: [Type<any>, Component]) => {
-            // Once resolved, we override the existing metadata, ensuring that the resolved
-            // resources
-            // are only available until the next TestBed reset (when `resetTestingModule` is called)
-            this.overrideComponent(override[0], {set: override[1]});
-          });
-        });
+    // If the component has no async resources (templateUrl, styleUrls), we can finish
+    // synchronously. This is important so that users who mistakenly treat `compileComponents`
+    // as synchronous don't encounter an error, as ViewEngine was tolerant of this.
+    if (!hasAsyncResources) {
+      overrideComponents();
+      return Promise.resolve();
+    } else {
+      let resourceLoader: ResourceLoader;
+      return resolveComponentResources(url => {
+               if (!resourceLoader) {
+                 resourceLoader = this.compilerInjector.get(ResourceLoader);
+               }
+               return Promise.resolve(resourceLoader.get(url));
+             })
+          .then(overrideComponents);
+    }
   }
 
   get(token: any, notFoundValue: any = Injector.THROW_IF_NOT_FOUND): any {