Skip to content

Commit c4c7509

Browse files
gkalpakalxhub
gkalpak
authored and
alxhub
committed
fix(upgrade): fix HMR for hybrid applications (#40045)
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 the downgraded module has been bootstrapped and there is at least one Angular component present on the page. The is due to a combination of two facts: - The logic for setting up the listener that destroys the AngularJS app depends on the downgraded module's `NgModuleRef`, which is only available after the module has been bootstrapped. - The [HMR dispose logic][3] depends on having an Angular element (identified by the auto-geenrated `ng-version` attribute) present in the DOM in order to retrieve the Angular `PlatformRef`. [1]: https://github.com/angular/angular-cli/blob/205ea2b638f154291993bfd9e065cd66ff20503/packages/angular_devkit/build_angular/src/webpack/plugins/hmr/hmr-accept.ts#L75 [2]: https://github.com/angular/angular-cli/blob/205ea2b638f154291993bfd9e065cd66ff205033/packages/angular_devkit/build_angular/src/webpack/plugins/hmr/hmr-accept.ts#L31 [3]: https://github.com/angular/angular-cli/blob/205ea2b638f154291993bfd9e065cd66ff205033/packages/angular_devkit/build_angular/src/webpack/plugins/hmr/hmr-accept.ts#L116 Fixes #39935 PR Close #40045
1 parent bed2e8a commit c4c7509

File tree

9 files changed

+238
-11
lines changed

9 files changed

+238
-11
lines changed

‎goldens/public-api/upgrade/static/static.d.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ export declare class UpgradeModule {
3535
ngZone: NgZone;
3636
constructor(
3737
injector: Injector,
38-
ngZone: NgZone);
38+
ngZone: NgZone,
39+
platformRef: PlatformRef);
3940
bootstrap(element: Element, modules?: string[], config?: any): void;
4041
}
4142

‎packages/upgrade/src/common/src/constants.ts

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export const $INJECTOR = '$injector';
1515
export const $INTERVAL = '$interval';
1616
export const $PARSE = '$parse';
1717
export const $PROVIDE = '$provide';
18+
export const $ROOT_ELEMENT = '$rootElement';
1819
export const $ROOT_SCOPE = '$rootScope';
1920
export const $SCOPE = '$scope';
2021
export const $TEMPLATE_CACHE = '$templateCache';

‎packages/upgrade/src/common/src/util.ts

+19-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88

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

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

1414
const DIRECTIVE_PREFIX_REGEXP = /^(?:x|data)[:\-_]/i;
1515
const DIRECTIVE_SPECIAL_CHARS_REGEXP = /[:\-_]+(.)/g;
@@ -48,6 +48,23 @@ export function controllerKey(name: string): string {
4848
return '$' + name + 'Controller';
4949
}
5050

51+
/**
52+
* Destroy an AngularJS app given the app `$injector`.
53+
*
54+
* NOTE: Destroying an app is not officially supported by AngularJS, but try to do our best by
55+
* destroying `$rootScope` and clean the jqLite/jQuery data on `$rootElement` and all
56+
* descendants.
57+
*
58+
* @param $injector The `$injector` of the AngularJS app to destroy.
59+
*/
60+
export function destroyApp($injector: IInjectorService): void {
61+
const $rootElement: IAugmentedJQuery = $injector.get($ROOT_ELEMENT);
62+
const $rootScope: IRootScopeService = $injector.get($ROOT_SCOPE);
63+
64+
$rootScope.$destroy();
65+
cleanData($rootElement[0]);
66+
}
67+
5168
export function directiveNormalize(name: string): string {
5269
return name.replace(DIRECTIVE_PREFIX_REGEXP, '')
5370
.replace(DIRECTIVE_SPECIAL_CHARS_REGEXP, (_, letter) => letter.toUpperCase());

‎packages/upgrade/src/dynamic/src/upgrade_adapter.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {bootstrap, element as angularElement, IAngularBootstrapConfig, IAugmente
1313
import {$$TESTABILITY, $COMPILE, $INJECTOR, $ROOT_SCOPE, COMPILER_KEY, INJECTOR_KEY, LAZY_MODULE_REF, NG_ZONE_KEY, UPGRADE_APP_TYPE_KEY} from '../../common/src/constants';
1414
import {downgradeComponent} from '../../common/src/downgrade_component';
1515
import {downgradeInjectable} from '../../common/src/downgrade_injectable';
16-
import {controllerKey, Deferred, LazyModuleRef, onError, UpgradeAppType} from '../../common/src/util';
16+
import {controllerKey, Deferred, destroyApp, LazyModuleRef, onError, UpgradeAppType} from '../../common/src/util';
1717

1818
import {UpgradeNg1ComponentAdapterBuilder} from './upgrade_ng1_adapter';
1919

@@ -619,6 +619,13 @@ export class UpgradeAdapter {
619619
rootScope.$on('$destroy', () => {
620620
subscription.unsubscribe();
621621
});
622+
623+
// Destroy the AngularJS app once the Angular `PlatformRef` is destroyed.
624+
// This does not happen in a typical SPA scenario, but it might be useful for
625+
// other use-cases where disposing of an Angular/AngularJS app is necessary
626+
// (such as Hot Module Replacement (HMR)).
627+
// See https://github.com/angular/angular/issues/39935.
628+
platformRef.onDestroy(() => destroyApp(ng1Injector));
622629
});
623630
})
624631
.catch((e) => this.ng2BootstrapDeferred.reject(e));

‎packages/upgrade/src/dynamic/test/upgrade_spec.ts

+59-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ withEachNg1Version(() => {
8686
});
8787
}));
8888

89-
it('supports the compilerOptions argument', waitForAsync(() => {
89+
it('should support the compilerOptions argument', waitForAsync(() => {
9090
const platformRef = platformBrowserDynamic();
9191
spyOn(platformRef, 'bootstrapModule').and.callThrough();
9292
spyOn(platformRef, 'bootstrapModuleFactory').and.callThrough();
@@ -120,6 +120,64 @@ withEachNg1Version(() => {
120120
ref.dispose();
121121
});
122122
}));
123+
124+
it('should destroy the AngularJS app when `PlatformRef` is destroyed', waitForAsync(() => {
125+
const platformRef = platformBrowserDynamic();
126+
const adapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
127+
const ng1Module = angular.module_('ng1', []);
128+
129+
@Component({selector: 'ng2', template: '<span>NG2</span>'})
130+
class Ng2Component {
131+
}
132+
133+
@NgModule({
134+
declarations: [Ng2Component],
135+
imports: [BrowserModule],
136+
})
137+
class Ng2Module {
138+
ngDoBootstrap() {}
139+
}
140+
141+
ng1Module.component('ng1', {template: '<ng2></ng2>'});
142+
ng1Module.directive('ng2', adapter.downgradeNg2Component(Ng2Component));
143+
144+
const element = html('<div><ng1></ng1></div>');
145+
146+
adapter.bootstrap(element, [ng1Module.name]).ready(ref => {
147+
const $rootScope: angular.IRootScopeService = ref.ng1Injector.get($ROOT_SCOPE);
148+
const rootScopeDestroySpy = spyOn($rootScope, '$destroy');
149+
150+
const appElem = angular.element(element);
151+
const ng1Elem = angular.element(element.querySelector('ng1') as Element);
152+
const ng2Elem = angular.element(element.querySelector('ng2') as Element);
153+
const ng2ChildElem = angular.element(element.querySelector('ng2 span') as Element);
154+
155+
// Attach data to all elements.
156+
appElem.data!('testData', 1);
157+
ng1Elem.data!('testData', 2);
158+
ng2Elem.data!('testData', 3);
159+
ng2ChildElem.data!('testData', 4);
160+
161+
// Verify data can be retrieved.
162+
expect(appElem.data!('testData')).toBe(1);
163+
expect(ng1Elem.data!('testData')).toBe(2);
164+
expect(ng2Elem.data!('testData')).toBe(3);
165+
expect(ng2ChildElem.data!('testData')).toBe(4);
166+
167+
expect(rootScopeDestroySpy).not.toHaveBeenCalled();
168+
169+
// Destroy `PlatformRef`.
170+
platformRef.destroy();
171+
172+
// Verify `$rootScope` has been destroyed and data has been cleaned up.
173+
expect(rootScopeDestroySpy).toHaveBeenCalled();
174+
175+
expect(appElem.data!('testData')).toBeUndefined();
176+
expect(ng1Elem.data!('testData')).toBeUndefined();
177+
expect(ng2Elem.data!('testData')).toBeUndefined();
178+
expect(ng2ChildElem.data!('testData')).toBeUndefined();
179+
});
180+
}));
123181
});
124182

125183
describe('bootstrap errors', () => {

‎packages/upgrade/static/src/downgrade_module.ts

+9-2
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

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

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

1616
import {angular1Providers, setTempInjectorRef} from './angular1_providers';
1717
import {NgAdapterInjector} from './util';
@@ -167,6 +167,13 @@ export function downgradeModule<T>(moduleFactoryOrBootstrapFn: NgModuleFactory<T
167167
injector = result.injector = new NgAdapterInjector(ref.injector);
168168
injector.get($INJECTOR);
169169

170+
// Destroy the AngularJS app once the Angular `PlatformRef` is destroyed.
171+
// This does not happen in a typical SPA scenario, but it might be useful for
172+
// other use-cases where disposing of an Angular/AngularJS app is necessary
173+
// (such as Hot Module Replacement (HMR)).
174+
// See https://github.com/angular/angular/issues/39935.
175+
injector.get(PlatformRef).onDestroy(() => destroyApp($injector));
176+
170177
return injector;
171178
})
172179
};

‎packages/upgrade/static/src/upgrade_module.ts

+17-4
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

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

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

1515
import {angular1Providers, setTempInjectorRef} from './angular1_providers';
1616
import {NgAdapterInjector} from './util';
@@ -155,7 +155,13 @@ export class UpgradeModule {
155155
/** The root `Injector` for the upgrade application. */
156156
injector: Injector,
157157
/** The bootstrap zone for the upgrade application */
158-
public ngZone: NgZone) {
158+
public ngZone: NgZone,
159+
/**
160+
* The owning `NgModuleRef`s `PlatformRef` instance.
161+
* This is used to tie the lifecycle of the bootstrapped AngularJS apps to that of the Angular
162+
* `PlatformRef`.
163+
*/
164+
private platformRef: PlatformRef) {
159165
this.injector = new NgAdapterInjector(injector);
160166
}
161167

@@ -242,6 +248,7 @@ export class UpgradeModule {
242248
$INJECTOR,
243249
($injector: IInjectorService) => {
244250
this.$injector = $injector;
251+
const $rootScope = $injector.get('$rootScope');
245252

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

260+
// Destroy the AngularJS app once the Angular `PlatformRef` is destroyed.
261+
// This does not happen in a typical SPA scenario, but it might be useful for
262+
// other use-cases where disposing of an Angular/AngularJS app is necessary
263+
// (such as Hot Module Replacement (HMR)).
264+
// See https://github.com/angular/angular/issues/39935.
265+
this.platformRef.onDestroy(() => destroyApp($injector));
266+
253267
// Wire up the ng1 rootScope to run a digest cycle whenever the zone settles
254268
// We need to do this in the next tick so that we don't prevent the bootup stabilizing
255269
setTimeout(() => {
256-
const $rootScope = $injector.get('$rootScope');
257270
const subscription = this.ngZone.onMicrotaskEmpty.subscribe(() => {
258271
if ($rootScope.$$phase) {
259272
if (isDevMode()) {

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

+61
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
1313
import {downgradeComponent, UpgradeComponent, UpgradeModule} from '@angular/upgrade/static';
1414

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

1819
import {$apply, bootstrap} from './static_test_helpers';
@@ -648,6 +649,66 @@ withEachNg1Version(() => {
648649
});
649650
}));
650651

652+
it('should destroy the AngularJS app when `PlatformRef` is destroyed', waitForAsync(() => {
653+
@Component({selector: 'ng2', template: '<span>NG2</span>'})
654+
class Ng2Component {
655+
}
656+
657+
@NgModule({
658+
declarations: [Ng2Component],
659+
entryComponents: [Ng2Component],
660+
imports: [BrowserModule, UpgradeModule],
661+
})
662+
class Ng2Module {
663+
ngDoBootstrap() {}
664+
}
665+
666+
const ng1Module = angular.module_('ng1', [])
667+
.component('ng1', {template: '<ng2></ng2>'})
668+
.directive('ng2', downgradeComponent({component: Ng2Component}));
669+
670+
const element = html('<div><ng1></ng1></div>');
671+
const platformRef = platformBrowserDynamic();
672+
673+
platformRef.bootstrapModule(Ng2Module).then(ref => {
674+
const upgrade = ref.injector.get(UpgradeModule);
675+
upgrade.bootstrap(element, [ng1Module.name]);
676+
677+
const $rootScope: angular.IRootScopeService = upgrade.$injector.get($ROOT_SCOPE);
678+
const rootScopeDestroySpy = spyOn($rootScope, '$destroy');
679+
680+
const appElem = angular.element(element);
681+
const ng1Elem = angular.element(element.querySelector('ng1') as Element);
682+
const ng2Elem = angular.element(element.querySelector('ng2') as Element);
683+
const ng2ChildElem = angular.element(element.querySelector('ng2 span') as Element);
684+
685+
// Attach data to all elements.
686+
appElem.data!('testData', 1);
687+
ng1Elem.data!('testData', 2);
688+
ng2Elem.data!('testData', 3);
689+
ng2ChildElem.data!('testData', 4);
690+
691+
// Verify data can be retrieved.
692+
expect(appElem.data!('testData')).toBe(1);
693+
expect(ng1Elem.data!('testData')).toBe(2);
694+
expect(ng2Elem.data!('testData')).toBe(3);
695+
expect(ng2ChildElem.data!('testData')).toBe(4);
696+
697+
expect(rootScopeDestroySpy).not.toHaveBeenCalled();
698+
699+
// Destroy `PlatformRef`.
700+
platformRef.destroy();
701+
702+
// Verify `$rootScope` has been destroyed and data has been cleaned up.
703+
expect(rootScopeDestroySpy).toHaveBeenCalled();
704+
705+
expect(appElem.data!('testData')).toBeUndefined();
706+
expect(ng1Elem.data!('testData')).toBeUndefined();
707+
expect(ng2Elem.data!('testData')).toBeUndefined();
708+
expect(ng2ChildElem.data!('testData')).toBeUndefined();
709+
});
710+
}));
711+
651712
it('should work when compiled outside the dom (by fallback to the root ng2.injector)',
652713
waitForAsync(() => {
653714
@Component({selector: 'ng2', template: 'test'})

‎packages/upgrade/static/test/integration/downgrade_module_spec.ts

+62
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,68 @@ withEachNg1Version(() => {
13531353
setTimeout(() => expect($injectorFromNg2).toBe($injectorFromNg1));
13541354
}));
13551355

1356+
it('should destroy the AngularJS app when `PlatformRef` is destroyed', waitForAsync(() => {
1357+
@Component({selector: 'ng2', template: '<span>NG2</span>'})
1358+
class Ng2Component {
1359+
}
1360+
1361+
@NgModule({
1362+
declarations: [Ng2Component],
1363+
entryComponents: [Ng2Component],
1364+
imports: [BrowserModule],
1365+
})
1366+
class Ng2Module {
1367+
ngDoBootstrap() {}
1368+
}
1369+
1370+
const bootstrapFn = (extraProviders: StaticProvider[]) =>
1371+
platformBrowserDynamic(extraProviders).bootstrapModule(Ng2Module);
1372+
const lazyModuleName = downgradeModule<Ng2Module>(bootstrapFn);
1373+
const ng1Module =
1374+
angular.module_('ng1', [lazyModuleName])
1375+
.component('ng1', {template: '<ng2></ng2>'})
1376+
.directive(
1377+
'ng2', downgradeComponent({component: Ng2Component, propagateDigest}));
1378+
1379+
const element = html('<div><ng1></ng1></div>');
1380+
const $injector = angular.bootstrap(element, [ng1Module.name]);
1381+
1382+
setTimeout(() => { // Wait for the module to be bootstrapped.
1383+
const $rootScope: angular.IRootScopeService = $injector.get($ROOT_SCOPE);
1384+
const rootScopeDestroySpy = spyOn($rootScope, '$destroy');
1385+
1386+
const appElem = angular.element(element);
1387+
const ng1Elem = angular.element(element.querySelector('ng1') as Element);
1388+
const ng2Elem = angular.element(element.querySelector('ng2') as Element);
1389+
const ng2ChildElem = angular.element(element.querySelector('ng2 span') as Element);
1390+
1391+
// Attach data to all elements.
1392+
appElem.data!('testData', 1);
1393+
ng1Elem.data!('testData', 2);
1394+
ng2Elem.data!('testData', 3);
1395+
ng2ChildElem.data!('testData', 4);
1396+
1397+
// Verify data can be retrieved.
1398+
expect(appElem.data!('testData')).toBe(1);
1399+
expect(ng1Elem.data!('testData')).toBe(2);
1400+
expect(ng2Elem.data!('testData')).toBe(3);
1401+
expect(ng2ChildElem.data!('testData')).toBe(4);
1402+
1403+
expect(rootScopeDestroySpy).not.toHaveBeenCalled();
1404+
1405+
// Destroy `PlatformRef`.
1406+
getPlatform()?.destroy();
1407+
1408+
// Verify `$rootScope` has been destroyed and data has been cleaned up.
1409+
expect(rootScopeDestroySpy).toHaveBeenCalled();
1410+
1411+
expect(appElem.data!('testData')).toBeUndefined();
1412+
expect(ng1Elem.data!('testData')).toBeUndefined();
1413+
expect(ng2Elem.data!('testData')).toBeUndefined();
1414+
expect(ng2ChildElem.data!('testData')).toBeUndefined();
1415+
});
1416+
}));
1417+
13561418
describe('(common error)', () => {
13571419
let Ng2CompA: Type<any>;
13581420
let Ng2CompB: Type<any>;

0 commit comments

Comments
 (0)