fix(upgrade): compile downgraded components synchronously (if possibl…

…e) (#31840)

AngularJS compilation is a synchronous operation (unless having to fetch
a template, which is not supported for downgraded components).
Previously, ngUpgrade tried to retain the synchronous nature of the
compilation for downgraded components (when possible), by using a
synchronous thenable implementation (`ParentInjectorPromise`). This was
accidentally broken in #27217 by replacing a call to
`ParentInjectorPromise#then()` (which can be synchronous) with a call to
`Promise.all()` (which is asynchronous).

This commit fixes this by introducing a `SyncPromise.all()` static
method; similar to `Promise.all()` but retaining the synchronous
capabilities of `SyncPromise` (which `ParentInjectorPromise` inherits
from).

Fixes #30330

PR Close #31840

packages/upgrade/src/common/src/downgrade_component.ts

@@ -196,12 +196,8 @@ export function downgradeComponent(info: {
               wrapCallback(() => doDowngrade(pInjector, mInjector))();
             };
 
-        if (isThenable(finalParentInjector) || isThenable(finalModuleInjector)) {
-          Promise.all([finalParentInjector, finalModuleInjector])
-              .then(([pInjector, mInjector]) => downgradeFn(pInjector, mInjector));
-        } else {
-          downgradeFn(finalParentInjector, finalModuleInjector);
-        }
+        ParentInjectorPromise.all([finalParentInjector, finalModuleInjector])
+            .then(([pInjector, mInjector]) => downgradeFn(pInjector, mInjector));
 
         ranAsync = true;
       }

packages/upgrade/src/common/src/promise_util.ts

@@ -22,6 +22,27 @@ export class SyncPromise<T> {
   private resolved = false;
   private callbacks: ((value: T) => unknown)[] = [];
 
+  static all<T>(valuesOrPromises: (T|Thenable<T>)[]): SyncPromise<T[]> {
+    const aggrPromise = new SyncPromise<T[]>();
+
+    let resolvedCount = 0;
+    const results: T[] = [];
+    const resolve = (idx: number, value: T) => {
+      results[idx] = value;
+      if (++resolvedCount === valuesOrPromises.length) aggrPromise.resolve(results);
+    };
+
+    valuesOrPromises.forEach((p, idx) => {
+      if (isThenable(p)) {
+        p.then(v => resolve(idx, v));
+      } else {
+        resolve(idx, p);
+      }
+    });
+
+    return aggrPromise;
+  }
+
   resolve(value: T): void {
     // Do nothing, if already resolved.
     if (this.resolved) return;

packages/upgrade/src/common/test/promise_util_spec.ts

@@ -85,4 +85,36 @@ describe('SyncPromise', () => {
     expect(spy).toHaveBeenCalledTimes(1);
     expect(spy).toHaveBeenCalledWith('foo');
   });
+
+  describe('.all()', () => {
+    it('should return a `SyncPromise` instance',
+       () => { expect(SyncPromise.all([])).toEqual(jasmine.any(SyncPromise)); });
+
+    it('should resolve immediately if the provided values are not thenable', () => {
+      const spy = jasmine.createSpy('spy');
+
+      const promise = SyncPromise.all(['foo', 1, {then: false}, []]);
+      promise.then(spy);
+
+      expect(spy).toHaveBeenCalledWith(['foo', 1, {then: false}, []]);
+    });
+
+    it('should wait for any thenables to resolve', async() => {
+      const spy = jasmine.createSpy('spy');
+
+      const v1 = 'foo';
+      const v2 = new SyncPromise<string>();
+      const v3 = Promise.resolve('baz');
+      const promise = SyncPromise.all([v1, v2, v3]);
+
+      promise.then(spy);
+      expect(spy).not.toHaveBeenCalled();
+
+      v2.resolve('bar');
+      expect(spy).not.toHaveBeenCalled();
+
+      await v3;
+      expect(spy).toHaveBeenCalledWith(['foo', 'bar', 'baz']);
+    });
+  });
 });

packages/upgrade/static/test/integration/downgrade_component_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ChangeDetectionStrategy, ChangeDetectorRef, Compiler, Component, ComponentFactoryResolver, Directive, ElementRef, EventEmitter, Injector, Input, NgModule, NgModuleRef, OnChanges, OnDestroy, Output, SimpleChanges, destroyPlatform} from '@angular/core';
+import {ChangeDetectionStrategy, Compiler, Component, Directive, ElementRef, EventEmitter, Injector, Input, NgModule, NgModuleRef, OnChanges, OnDestroy, Output, SimpleChanges, destroyPlatform} from '@angular/core';
 import {async, fakeAsync, tick} from '@angular/core/testing';
 import {BrowserModule} from '@angular/platform-browser';
 import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
@@ -736,6 +736,35 @@ withEachNg1Version(() => {
          });
        }));
 
+    it('should be compiled synchronously, if possible', async(() => {
+         @Component({selector: 'ng2A', template: '<ng-content></ng-content>'})
+         class Ng2ComponentA {
+         }
+
+         @Component({selector: 'ng2B', template: '{{ \'Ng2 template\' }}'})
+         class Ng2ComponentB {
+         }
+
+         @NgModule({
+           declarations: [Ng2ComponentA, Ng2ComponentB],
+           entryComponents: [Ng2ComponentA, Ng2ComponentB],
+           imports: [BrowserModule, UpgradeModule],
+         })
+         class Ng2Module {
+           ngDoBootstrap() {}
+         }
+
+         const ng1Module = angular.module_('ng1', [])
+                               .directive('ng2A', downgradeComponent({component: Ng2ComponentA}))
+                               .directive('ng2B', downgradeComponent({component: Ng2ComponentB}));
+
+         const element = html('<ng2-a><ng2-b></ng2-b></ng2-a>');
+
+         bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(() => {
+           expect(element.textContent).toBe('Ng2 template');
+         });
+       }));
+
     it('should work with ng2 lazy loaded components', async(() => {
          let componentInjector: Injector;