Skip to content

Commit

Permalink
fix(upgrade): correctly handle nested downgraded components with `dow…
Browse files Browse the repository at this point in the history
…ngradeModule()`

Previously, nested downgraded components would not be created/destroyed
inside the Angular zone (as they should) and they should not be wired up
correctly for change detection.

This commit ensures that ngUpgrade correctly detects whether this is an
ngUpgradeLite app (i.e. one using `downgradeModule()` instead of
`UpgradeModule`) and appropriately handles components, even if they are
nested inside other downgraded components.

Fixes angular#22581
Closes angular#22869
Closes angular#27083
  • Loading branch information
gkalpak committed Nov 22, 2018
1 parent e5b4912 commit 482b400
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 6 deletions.
8 changes: 3 additions & 5 deletions packages/upgrade/src/common/downgrade_component.ts
Expand Up @@ -86,8 +86,9 @@ export function downgradeComponent(info: {
// NOTE: This is not needed, when using `UpgradeModule`, because `$digest()` will be run
// inside the Angular zone (except if explicitly escaped, in which case we shouldn't
// force it back in).
let isNgUpgradeLite = false;
let wrapCallback = <T>(cb: () => T) => cb;
const isNgUpgradeLite = getUpgradeAppType($injector) === UpgradeAppType.Lite;
const wrapCallback: <T>(cb: () => T) => typeof cb =
!isNgUpgradeLite ? cb => cb : cb => () => NgZone.isInAngularZone() ? cb() : ngZone.run(cb);
let ngZone: NgZone;

return {
Expand All @@ -112,7 +113,6 @@ export function downgradeComponent(info: {
validateInjectionKey($injector, downgradedModule, lazyModuleRefKey, attemptedAction);

const lazyModuleRef = $injector.get(lazyModuleRefKey) as LazyModuleRef;
isNgUpgradeLite = getUpgradeAppType($injector) === UpgradeAppType.Lite;
parentInjector = lazyModuleRef.injector || lazyModuleRef.promise as Promise<Injector>;
}

Expand Down Expand Up @@ -149,8 +149,6 @@ export function downgradeComponent(info: {
const downgradeFn = !isNgUpgradeLite ? doDowngrade : (injector: Injector) => {
if (!ngZone) {
ngZone = injector.get(NgZone);
wrapCallback = <T>(cb: () => T) => () =>
NgZone.isInAngularZone() ? cb() : ngZone.run(cb);
}

wrapCallback(() => doDowngrade(injector))();
Expand Down
121 changes: 120 additions & 1 deletion packages/upgrade/test/static/integration/downgrade_module_spec.ts
Expand Up @@ -316,11 +316,73 @@ withEachNg1Version(() => {
});
}));

it('should create and destroy nested, asynchronously instantiated components inside the Angular zone',
async(() => {
let createdInTheZone = false;
let destroyedInTheZone = false;

@Component({
selector: 'test',
template: '',
})
class TestComponent implements OnDestroy {
constructor() { createdInTheZone = NgZone.isInAngularZone(); }
ngOnDestroy() { destroyedInTheZone = NgZone.isInAngularZone(); }
}

@Component({
selector: 'wrapper',
template: '<ng-content></ng-content>',
})
class WrapperComponent {
}

@NgModule({
declarations: [TestComponent, WrapperComponent],
entryComponents: [TestComponent, WrapperComponent],
imports: [BrowserModule],
})
class Ng2Module {
ngDoBootstrap() {}
}

const bootstrapFn = (extraProviders: StaticProvider[]) =>
platformBrowserDynamic(extraProviders).bootstrapModule(Ng2Module);
const lazyModuleName = downgradeModule<Ng2Module>(bootstrapFn);
const ng1Module =
angular.module('ng1', [lazyModuleName])
.directive(
'test', downgradeComponent({component: TestComponent, propagateDigest}))
.directive(
'wrapper',
downgradeComponent({component: WrapperComponent, propagateDigest}));

// Important: `ng-if` makes `<zonetest>` render asynchronously.
const element = html('<wrapper><test ng-if="showNg2"></test></wrapper>');
const $injector = angular.bootstrap(element, [ng1Module.name]);
const $rootScope = $injector.get($ROOT_SCOPE) as angular.IRootScopeService;

// Wait for the module to be bootstrapped.
setTimeout(() => {
// Create nested component asynchronously.
expect(createdInTheZone).toBe(false);

$rootScope.$apply('showNg2 = true');
expect(createdInTheZone).toBe(true);

// Destroy nested component asynchronously.
expect(destroyedInTheZone).toBe(false);

$rootScope.$apply('showNg2 = false');
expect(destroyedInTheZone).toBe(true);
});
}));

it('should wire up the component for change detection', async(() => {
@Component(
{selector: 'ng2', template: '{{ count }}<button (click)="increment()"></button>'})
class Ng2Component {
private count = 0;
count = 0;
increment() { ++this.count; }
}

Expand Down Expand Up @@ -358,6 +420,63 @@ withEachNg1Version(() => {
});
}));

it('should wire up nested, asynchronously instantiated components for change detection',
async(() => {
@Component(
{selector: 'test', template: '{{ count }}<button (click)="increment()"></button>'})
class TestComponent {
count = 0;
increment() { ++this.count; }
}

@Component({
selector: 'wrapper',
template: '<ng-content></ng-content>',
})
class WrapperComponent {
}

@NgModule({
declarations: [TestComponent, WrapperComponent],
entryComponents: [TestComponent, WrapperComponent],
imports: [BrowserModule],
})
class Ng2Module {
ngDoBootstrap() {}
}

const bootstrapFn = (extraProviders: StaticProvider[]) =>
platformBrowserDynamic(extraProviders).bootstrapModule(Ng2Module);
const lazyModuleName = downgradeModule<Ng2Module>(bootstrapFn);
const ng1Module =
angular.module('ng1', [lazyModuleName])
.directive(
'test', downgradeComponent({component: TestComponent, propagateDigest}))
.directive(
'wrapper',
downgradeComponent({component: WrapperComponent, propagateDigest}));

// Important: `ng-if` makes `<zonetest>` render asynchronously.
const element = html('<wrapper><test ng-if="showNg2"></test></wrapper>');
const $injector = angular.bootstrap(element, [ng1Module.name]);
const $rootScope = $injector.get($ROOT_SCOPE) as angular.IRootScopeService;

// Wait for the module to be bootstrapped.
setTimeout(() => {
// Create nested component asynchronously.
$rootScope.$apply('showNg2 = true');
const button = element.querySelector('button') !;

expect(element.textContent).toBe('0');

button.click();
expect(element.textContent).toBe('1');

button.click();
expect(element.textContent).toBe('2');
});
}));

it('should run the lifecycle hooks in the correct order', async(() => {
const logs: string[] = [];
let rootScope: angular.IRootScopeService;
Expand Down

0 comments on commit 482b400

Please sign in to comment.