fix(upgrade): compile downgraded components synchronously (if possible) (#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
This commit is contained in:
George Kalpakas 2019-07-25 15:38:08 +03:00 committed by Alex Rickabaugh
parent b3b5c66414
commit c1ae6124c8
4 changed files with 85 additions and 7 deletions

View File

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

View File

@ -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;

View File

@ -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']);
});
});
});

View File

@ -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;