Skip to content

Commit

Permalink
fix(upgrade): fix HMR for hybrid applications
Browse files Browse the repository at this point in the history
Previously, trying to apply a change via Hot Module Replacement (HMR) in
a hybrid app would result in an error. This was caused by not having the
AngularJS app destroyed and thus trying to bootstrap an AngularJS app on
the same element twice.

This commit fixes HMR for hybrid apps by ensuring the AngularJS app is
destroyed when the Angular `PlatformRef` is [destroyed][1] in the
[`module.hot.dispose()` callback][2].

NOTE:
For "ngUpgradeLite" apps (i.e. those using `downgradeModule()`), HMR
will only work if there is at least one Angular component present on the
page.

[1]:
https://github.com/angular/angular-cli/blob/d3afdcc1b3e40736dab3ca7ae81c462747eb0ac9/packages/angular_devkit/build_angular/src/webpack/plugins/hmr/hmr-accept.ts#L75
[2]:
https://github.com/angular/angular-cli/blob/d3afdcc1b3e40736dab3ca7ae81c462747eb0ac9/packages/angular_devkit/build_angular/src/webpack/plugins/hmr/hmr-accept.ts#L31

Fixes #39935
  • Loading branch information
gkalpak committed Dec 9, 2020
1 parent 43bb75d commit 204e21e
Show file tree
Hide file tree
Showing 9 changed files with 229 additions and 11 deletions.
3 changes: 2 additions & 1 deletion goldens/public-api/upgrade/static/static.d.ts
Expand Up @@ -35,7 +35,8 @@ export declare class UpgradeModule {
ngZone: NgZone;
constructor(
injector: Injector,
ngZone: NgZone);
ngZone: NgZone,
platformRef: PlatformRef);
bootstrap(element: Element, modules?: string[], config?: any): void;
}

Expand Down
1 change: 1 addition & 0 deletions packages/upgrade/src/common/src/constants.ts
Expand Up @@ -15,6 +15,7 @@ export const $INJECTOR = '$injector';
export const $INTERVAL = '$interval';
export const $PARSE = '$parse';
export const $PROVIDE = '$provide';
export const $ROOT_ELEMENT = '$rootElement';
export const $ROOT_SCOPE = '$rootScope';
export const $SCOPE = '$scope';
export const $TEMPLATE_CACHE = '$templateCache';
Expand Down
15 changes: 13 additions & 2 deletions packages/upgrade/src/common/src/util.ts
Expand Up @@ -8,8 +8,8 @@

import {Injector, Type} from '@angular/core';

import {element as angularElement, IInjectorService, INgModelController} from './angular1';
import {DOWNGRADED_MODULE_COUNT_KEY, UPGRADE_APP_TYPE_KEY} from './constants';
import {element as angularElement, IAugmentedJQuery, IInjectorService, INgModelController, IRootScopeService} from './angular1';
import {$ROOT_ELEMENT, $ROOT_SCOPE, DOWNGRADED_MODULE_COUNT_KEY, UPGRADE_APP_TYPE_KEY} from './constants';

const DIRECTIVE_PREFIX_REGEXP = /^(?:x|data)[:\-_]/i;
const DIRECTIVE_SPECIAL_CHARS_REGEXP = /[:\-_]+(.)/g;
Expand Down Expand Up @@ -44,6 +44,17 @@ export function controllerKey(name: string): string {
return '$' + name + 'Controller';
}

// Destroy an AngularJS app given the app `$injector`.
//
// NOTE: Destroying an app is not officially supported by AngularJS, but we do our best.
export function destroyApp($injector: IInjectorService): void {
const $rootElement: IAugmentedJQuery = $injector.get($ROOT_ELEMENT);
const $rootScope: IRootScopeService = $injector.get($ROOT_SCOPE);

$rootScope.$destroy();
cleanData($rootElement[0]);
}

export function directiveNormalize(name: string): string {
return name.replace(DIRECTIVE_PREFIX_REGEXP, '')
.replace(DIRECTIVE_SPECIAL_CHARS_REGEXP, (_, letter) => letter.toUpperCase());
Expand Down
8 changes: 7 additions & 1 deletion packages/upgrade/src/dynamic/src/upgrade_adapter.ts
Expand Up @@ -13,7 +13,7 @@ import {bootstrap, element as angularElement, IAngularBootstrapConfig, IAugmente
import {$$TESTABILITY, $COMPILE, $INJECTOR, $ROOT_SCOPE, COMPILER_KEY, INJECTOR_KEY, LAZY_MODULE_REF, NG_ZONE_KEY, UPGRADE_APP_TYPE_KEY} from '../../common/src/constants';
import {downgradeComponent} from '../../common/src/downgrade_component';
import {downgradeInjectable} from '../../common/src/downgrade_injectable';
import {controllerKey, Deferred, LazyModuleRef, onError, UpgradeAppType} from '../../common/src/util';
import {controllerKey, Deferred, destroyApp, LazyModuleRef, onError, UpgradeAppType} from '../../common/src/util';

import {UpgradeNg1ComponentAdapterBuilder} from './upgrade_ng1_adapter';

Expand Down Expand Up @@ -619,6 +619,12 @@ export class UpgradeAdapter {
rootScope.$on('$destroy', () => {
subscription.unsubscribe();
});

// Destroy the AngularJS app once the Angular `PlatformRef` is destroyed.
// This does not happen in a typical SPA scenario, but it might be useful for
// other usecases where desposing of an Angular/AngularJS app is necessary (such
// as Hot Module Replacement (HMR)).
platformRef.onDestroy(() => destroyApp(ng1Injector));
});
})
.catch((e) => this.ng2BootstrapDeferred.reject(e));
Expand Down
60 changes: 59 additions & 1 deletion packages/upgrade/src/dynamic/test/upgrade_spec.ts
Expand Up @@ -86,7 +86,7 @@ withEachNg1Version(() => {
});
}));

it('supports the compilerOptions argument', waitForAsync(() => {
it('should support the compilerOptions argument', waitForAsync(() => {
const platformRef = platformBrowserDynamic();
spyOn(platformRef, 'bootstrapModule').and.callThrough();
spyOn(platformRef, 'bootstrapModuleFactory').and.callThrough();
Expand Down Expand Up @@ -120,6 +120,64 @@ withEachNg1Version(() => {
ref.dispose();
});
}));

it('should destroy the AngularJS app when `PlatformRef` is destroyed', waitForAsync(() => {
const platformRef = platformBrowserDynamic();
const adapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
const ng1Module = angular.module_('ng1', []);

@Component({selector: 'ng2', template: '<span>NG2</span>'})
class Ng2Component {
}

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

ng1Module.component('ng1', {template: '<ng2></ng2>'});
ng1Module.directive('ng2', adapter.downgradeNg2Component(Ng2Component));

const element = html('<div><ng1></ng1></div>');

adapter.bootstrap(element, [ng1Module.name]).ready(ref => {
const $rootScope: angular.IRootScopeService = ref.ng1Injector.get($ROOT_SCOPE);
const rootScopeDestroySpy = spyOn($rootScope, '$destroy');

const appElem = angular.element(element);
const ng1Elem = angular.element(element.querySelector('ng1') as Element);
const ng2Elem = angular.element(element.querySelector('ng2') as Element);
const ng2ChildElem = angular.element(element.querySelector('ng2 span') as Element);

// Attach data to all elements.
appElem.data!('testData', 1);
ng1Elem.data!('testData', 2);
ng2Elem.data!('testData', 3);
ng2ChildElem.data!('testData', 4);

// Verify data can be retrieved.
expect(appElem.data!('testData')).toBe(1);
expect(ng1Elem.data!('testData')).toBe(2);
expect(ng2Elem.data!('testData')).toBe(3);
expect(ng2ChildElem.data!('testData')).toBe(4);

expect(rootScopeDestroySpy).not.toHaveBeenCalled();

// Destroy `PlatformRef`.
platformRef.destroy();

// Verify `$rootScope` has been destroyed and data has been cleaned up.
expect(rootScopeDestroySpy).toHaveBeenCalled();

expect(appElem.data!('testData')).toBeUndefined();
expect(ng1Elem.data!('testData')).toBeUndefined();
expect(ng2Elem.data!('testData')).toBeUndefined();
expect(ng2ChildElem.data!('testData')).toBeUndefined();
});
}));
});

describe('bootstrap errors', () => {
Expand Down
10 changes: 8 additions & 2 deletions packages/upgrade/static/src/downgrade_module.ts
Expand Up @@ -6,12 +6,12 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Injector, NgModuleFactory, NgModuleRef, StaticProvider} from '@angular/core';
import {Injector, NgModuleFactory, NgModuleRef, PlatformRef, StaticProvider} from '@angular/core';
import {platformBrowser} from '@angular/platform-browser';

import {IInjectorService, IProvideService, module_ as angularModule} from '../../src/common/src/angular1';
import {$INJECTOR, $PROVIDE, DOWNGRADED_MODULE_COUNT_KEY, INJECTOR_KEY, LAZY_MODULE_REF, UPGRADE_APP_TYPE_KEY, UPGRADE_MODULE_NAME} from '../../src/common/src/constants';
import {getDowngradedModuleCount, isFunction, LazyModuleRef, UpgradeAppType} from '../../src/common/src/util';
import {destroyApp, getDowngradedModuleCount, isFunction, LazyModuleRef, UpgradeAppType} from '../../src/common/src/util';

import {angular1Providers, setTempInjectorRef} from './angular1_providers';
import {NgAdapterInjector} from './util';
Expand Down Expand Up @@ -167,6 +167,12 @@ export function downgradeModule<T>(moduleFactoryOrBootstrapFn: NgModuleFactory<T
injector = result.injector = new NgAdapterInjector(ref.injector);
injector.get($INJECTOR);

// Destroy the AngularJS app once the Angular `PlatformRef` is destroyed.
// This does not happen in a typical SPA scenario, but it might be useful for
// other usecases where desposing of an Angular/AngularJS app is necessary (such
// as Hot Module Replacement (HMR)).
injector.get(PlatformRef).onDestroy(() => destroyApp($injector));

return injector;
})
};
Expand Down
20 changes: 16 additions & 4 deletions packages/upgrade/static/src/upgrade_module.ts
Expand Up @@ -6,11 +6,11 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Injector, isDevMode, NgModule, NgZone, Testability} from '@angular/core';
import {Injector, isDevMode, NgModule, NgZone, PlatformRef, Testability} from '@angular/core';

import {bootstrap, element as angularElement, IInjectorService, IIntervalService, IProvideService, ITestabilityService, module_ as angularModule} from '../../src/common/src/angular1';
import {$$TESTABILITY, $DELEGATE, $INJECTOR, $INTERVAL, $PROVIDE, INJECTOR_KEY, LAZY_MODULE_REF, UPGRADE_APP_TYPE_KEY, UPGRADE_MODULE_NAME} from '../../src/common/src/constants';
import {controllerKey, LazyModuleRef, UpgradeAppType} from '../../src/common/src/util';
import {controllerKey, destroyApp, LazyModuleRef, UpgradeAppType} from '../../src/common/src/util';

import {angular1Providers, setTempInjectorRef} from './angular1_providers';
import {NgAdapterInjector} from './util';
Expand Down Expand Up @@ -155,7 +155,13 @@ export class UpgradeModule {
/** The root `Injector` for the upgrade application. */
injector: Injector,
/** The bootstrap zone for the upgrade application */
public ngZone: NgZone) {
public ngZone: NgZone,
/**
* The owning `NgModuleRef`s `PlatformRef` instance.
* This is used to tie the lifecycle of the bootstrapped AngularJS apps to that of the Angular
* `PlatformRef`.
*/
private platformRef: PlatformRef) {
this.injector = new NgAdapterInjector(injector);
}

Expand Down Expand Up @@ -242,6 +248,7 @@ export class UpgradeModule {
$INJECTOR,
($injector: IInjectorService) => {
this.$injector = $injector;
const $rootScope = $injector.get('$rootScope');

// Initialize the ng1 $injector provider
setTempInjectorRef($injector);
Expand All @@ -250,10 +257,15 @@ export class UpgradeModule {
// Put the injector on the DOM, so that it can be "required"
angularElement(element).data!(controllerKey(INJECTOR_KEY), this.injector);

// Destroy the AngularJS app once the Angular `PlatformRef` is destroyed.
// This does not happen in a typical SPA scenario, but it might be useful for
// other usecases where desposing of an Angular/AngularJS app is necessary (such
// as Hot Module Replacement (HMR)).
this.platformRef.onDestroy(() => destroyApp($injector));

// Wire up the ng1 rootScope to run a digest cycle whenever the zone settles
// We need to do this in the next tick so that we don't prevent the bootup stabilizing
setTimeout(() => {
const $rootScope = $injector.get('$rootScope');
const subscription = this.ngZone.onMicrotaskEmpty.subscribe(() => {
if ($rootScope.$$phase) {
if (isDevMode()) {
Expand Down
Expand Up @@ -13,6 +13,7 @@ import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
import {downgradeComponent, UpgradeComponent, UpgradeModule} from '@angular/upgrade/static';

import * as angular from '../../../src/common/src/angular1';
import {$ROOT_SCOPE} from '../../../src/common/src/constants';
import {html, multiTrim, withEachNg1Version} from '../../../src/common/test/helpers/common_test_helpers';

import {$apply, bootstrap} from './static_test_helpers';
Expand Down Expand Up @@ -648,6 +649,66 @@ withEachNg1Version(() => {
});
}));

it('should destroy the AngularJS app when `PlatformRef` is destroyed', waitForAsync(() => {
@Component({selector: 'ng2', template: '<span>NG2</span>'})
class Ng2Component {
}

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

const ng1Module = angular.module_('ng1', [])
.component('ng1', {template: '<ng2></ng2>'})
.directive('ng2', downgradeComponent({component: Ng2Component}));

const element = html('<div><ng1></ng1></div>');
const platformRef = platformBrowserDynamic();

platformRef.bootstrapModule(Ng2Module).then(ref => {
const upgrade = ref.injector.get(UpgradeModule);
upgrade.bootstrap(element, [ng1Module.name]);

const $rootScope: angular.IRootScopeService = upgrade.$injector.get($ROOT_SCOPE);
const rootScopeDestroySpy = spyOn($rootScope, '$destroy');

const appElem = angular.element(element);
const ng1Elem = angular.element(element.querySelector('ng1') as Element);
const ng2Elem = angular.element(element.querySelector('ng2') as Element);
const ng2ChildElem = angular.element(element.querySelector('ng2 span') as Element);

// Attach data to all elements.
appElem.data!('testData', 1);
ng1Elem.data!('testData', 2);
ng2Elem.data!('testData', 3);
ng2ChildElem.data!('testData', 4);

// Verify data can be retrieved.
expect(appElem.data!('testData')).toBe(1);
expect(ng1Elem.data!('testData')).toBe(2);
expect(ng2Elem.data!('testData')).toBe(3);
expect(ng2ChildElem.data!('testData')).toBe(4);

expect(rootScopeDestroySpy).not.toHaveBeenCalled();

// Destroy `PlatformRef`.
platformRef.destroy();

// Verify `$rootScope` has been destroyed and data has been cleaned up.
expect(rootScopeDestroySpy).toHaveBeenCalled();

expect(appElem.data!('testData')).toBeUndefined();
expect(ng1Elem.data!('testData')).toBeUndefined();
expect(ng2Elem.data!('testData')).toBeUndefined();
expect(ng2ChildElem.data!('testData')).toBeUndefined();
});
}));

it('should work when compiled outside the dom (by fallback to the root ng2.injector)',
waitForAsync(() => {
@Component({selector: 'ng2', template: 'test'})
Expand Down
62 changes: 62 additions & 0 deletions packages/upgrade/static/test/integration/downgrade_module_spec.ts
Expand Up @@ -1353,6 +1353,68 @@ withEachNg1Version(() => {
setTimeout(() => expect($injectorFromNg2).toBe($injectorFromNg1));
}));

it('should destroy the AngularJS app when `PlatformRef` is destroyed', waitForAsync(() => {
@Component({selector: 'ng2', template: '<span>NG2</span>'})
class Ng2Component {
}

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

const bootstrapFn = (extraProviders: StaticProvider[]) =>
platformBrowserDynamic(extraProviders).bootstrapModule(Ng2Module);
const lazyModuleName = downgradeModule<Ng2Module>(bootstrapFn);
const ng1Module =
angular.module_('ng1', [lazyModuleName])
.component('ng1', {template: '<ng2></ng2>'})
.directive(
'ng2', downgradeComponent({component: Ng2Component, propagateDigest}));

const element = html('<div><ng1></ng1></div>');
const $injector = angular.bootstrap(element, [ng1Module.name]);

setTimeout(() => { // Wait for the module to be bootstrapped.
const $rootScope: angular.IRootScopeService = $injector.get($ROOT_SCOPE);
const rootScopeDestroySpy = spyOn($rootScope, '$destroy');

const appElem = angular.element(element);
const ng1Elem = angular.element(element.querySelector('ng1') as Element);
const ng2Elem = angular.element(element.querySelector('ng2') as Element);
const ng2ChildElem = angular.element(element.querySelector('ng2 span') as Element);

// Attach data to all elements.
appElem.data!('testData', 1);
ng1Elem.data!('testData', 2);
ng2Elem.data!('testData', 3);
ng2ChildElem.data!('testData', 4);

// Verify data can be retrieved.
expect(appElem.data!('testData')).toBe(1);
expect(ng1Elem.data!('testData')).toBe(2);
expect(ng2Elem.data!('testData')).toBe(3);
expect(ng2ChildElem.data!('testData')).toBe(4);

expect(rootScopeDestroySpy).not.toHaveBeenCalled();

// Destroy `PlatformRef`.
getPlatform()?.destroy();

// Verify `$rootScope` has been destroyed and data has been cleaned up.
expect(rootScopeDestroySpy).toHaveBeenCalled();

expect(appElem.data!('testData')).toBeUndefined();
expect(ng1Elem.data!('testData')).toBeUndefined();
expect(ng2Elem.data!('testData')).toBeUndefined();
expect(ng2ChildElem.data!('testData')).toBeUndefined();
});
}));

describe('(common error)', () => {
let Ng2CompA: Type<any>;
let Ng2CompB: Type<any>;
Expand Down

0 comments on commit 204e21e

Please sign in to comment.