Skip to content
Permalink
Browse files

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
  • Loading branch information...
gkalpak authored and alxhub committed Jul 25, 2019
1 parent 6129cfa commit 04ebd599613573728661f61995bcf063604ae28c
@@ -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;
}
@@ -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;
@@ -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']);
});
});
});
@@ -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;

0 comments on commit 04ebd59

Please sign in to comment.
You can’t perform that action at this time.