Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(upgrade): fix HMR for hybrid applications #40045

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
11 changes: 3 additions & 8 deletions packages/upgrade/src/common/src/downgrade_component_adapter.ts
Expand Up @@ -8,10 +8,10 @@

import {ApplicationRef, ChangeDetectorRef, ComponentFactory, ComponentRef, EventEmitter, Injector, OnChanges, SimpleChange, SimpleChanges, StaticProvider, Testability, TestabilityRegistry, Type} from '@angular/core';

import {element as angularElement, IAttributes, IAugmentedJQuery, ICompileService, INgModelController, IParseService, IScope} from './angular1';
import {IAttributes, IAugmentedJQuery, ICompileService, INgModelController, IParseService, IScope} from './angular1';
import {PropertyBinding} from './component_info';
import {$SCOPE} from './constants';
import {getTypeName, hookupNgModel, strictEquals} from './util';
import {cleanData, getTypeName, hookupNgModel, strictEquals} from './util';

const INITIAL_VALUE = {
__UNINITIALIZED__: true
Expand Down Expand Up @@ -241,12 +241,7 @@ export class DowngradeComponentAdapter {
//
// To ensure the element is always properly cleaned up, we manually call `cleanData()` on
// this element and its descendants before destroying the `ComponentRef`.
//
// NOTE:
// `cleanData()` also will invoke the AngularJS `$destroy` event on the element:
// https://github.com/angular/angular.js/blob/2e72ea13fa98bebf6ed4b5e3c45eaf5f990ed16f/src/Angular.js#L1932-L1945
angularElement.cleanData(this.element);
angularElement.cleanData((this.element[0] as Element).querySelectorAll('*'));
cleanData(this.element[0]);

destroyComponentRef();
}
Expand Down
12 changes: 2 additions & 10 deletions packages/upgrade/src/common/src/upgrade_helper.ts
Expand Up @@ -10,7 +10,7 @@ import {ElementRef, Injector, SimpleChanges} from '@angular/core';

import {DirectiveRequireProperty, element as angularElement, IAugmentedJQuery, ICloneAttachFunction, ICompileService, IController, IControllerService, IDirective, IHttpBackendService, IInjectorService, ILinkFn, IScope, ITemplateCacheService, SingleOrListOrMap} from './angular1';
import {$COMPILE, $CONTROLLER, $HTTP_BACKEND, $INJECTOR, $TEMPLATE_CACHE} from './constants';
import {controllerKey, directiveNormalize, isFunction} from './util';
import {cleanData, controllerKey, directiveNormalize, isFunction} from './util';



Expand Down Expand Up @@ -125,15 +125,7 @@ export class UpgradeHelper {
controllerInstance.$onDestroy();
}
$scope.$destroy();

// Clean the jQuery/jqLite data on the component+child elements.
// Equivelent to how jQuery/jqLite invoke `cleanData` on an Element (this.element)
// https://github.com/jquery/jquery/blob/e743cbd28553267f955f71ea7248377915613fd9/src/manipulation.js#L223
// https://github.com/angular/angular.js/blob/26ddc5f830f902a3d22f4b2aab70d86d4d688c82/src/jqLite.js#L306-L312
// `cleanData` will invoke the AngularJS `$destroy` DOM event
// https://github.com/angular/angular.js/blob/26ddc5f830f902a3d22f4b2aab70d86d4d688c82/src/Angular.js#L1911-L1924
angularElement.cleanData([this.element]);
angularElement.cleanData(this.element.querySelectorAll('*'));
cleanData(this.element);
}

prepareTransclusion(): ILinkFn|undefined {
Expand Down
44 changes: 42 additions & 2 deletions packages/upgrade/src/common/src/util.ts
Expand Up @@ -8,8 +8,8 @@

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

import {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 All @@ -25,10 +25,46 @@ export function onError(e: any) {
throw e;
}

/**
* Clean the jqLite/jQuery data on the element and all its descendants.
* Equivalent to how jqLite/jQuery invoke `cleanData()` on an Element when removed:
* https://github.com/angular/angular.js/blob/2e72ea13fa98bebf6ed4b5e3c45eaf5f990ed16f/src/jqLite.js#L349-L355
* https://github.com/jquery/jquery/blob/6984d1747623dbc5e87fd6c261a5b6b1628c107c/src/manipulation.js#L182
*
* NOTE:
* `cleanData()` will also invoke the AngularJS `$destroy` DOM event on the element:
* https://github.com/angular/angular.js/blob/2e72ea13fa98bebf6ed4b5e3c45eaf5f990ed16f/src/Angular.js#L1932-L1945
*
* @param node The DOM node whose data needs to be cleaned.
*/
export function cleanData(node: Node): void {
angularElement.cleanData([node]);
if (isParentNode(node)) {
angularElement.cleanData(node.querySelectorAll('*'));
}
}

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 try to do our best by
* destroying `$rootScope` and clean the jqLite/jQuery data on `$rootElement` and all
* descendants.
*
* @param $injector The `$injector` of the AngularJS app to destroy.
*/
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 All @@ -53,6 +89,10 @@ export function isFunction(value: any): value is Function {
return typeof value === 'function';
}

function isParentNode(node: Node|ParentNode): node is ParentNode {
return isFunction((node as unknown as ParentNode).querySelectorAll);
}

export function validateInjectionKey(
$injector: IInjectorService, downgradedModule: string, injectionKey: string,
attemptedAction: string): void {
Expand Down
12 changes: 9 additions & 3 deletions 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 @@ -505,7 +505,6 @@ export class UpgradeAdapter {
const delayApplyExps: Function[] = [];
let original$applyFn: Function;
let rootScopePrototype: any;
let rootScope: IRootScopeService;
const upgradeAdapter = this;
const ng1Module = this.ng1Module = angularModule(this.idPrefix, modules);
const platformRef = platformBrowserDynamic();
Expand Down Expand Up @@ -533,7 +532,7 @@ export class UpgradeAdapter {
} else {
throw new Error('Failed to find \'$apply\' on \'$rootScope\'!');
}
return rootScope = rootScopeDelegate;
return rootScopeDelegate;
}
]);
if (ng1Injector.has($$TESTABILITY)) {
Expand Down Expand Up @@ -620,6 +619,13 @@ 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 use-cases where disposing of an Angular/AngularJS app is necessary
// (such as Hot Module Replacement (HMR)).
// See https://github.com/angular/angular/issues/39935.
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(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

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
11 changes: 9 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,13 @@ 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 use-cases where disposing of an Angular/AngularJS app is necessary
// (such as Hot Module Replacement (HMR)).
// See https://github.com/angular/angular/issues/39935.
injector.get(PlatformRef).onDestroy(() => destroyApp($injector));

return injector;
})
};
Expand Down