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

gkalpak · 2020-12-09T15:38:33Z

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 in the module.hot.dispose() callback.

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 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.

Fixes #39935.

The text was updated successfully, but these errors were encountered:

mary-poppins · 2020-12-09T15:50:14Z

You can preview 204e21e at https://pr40045-204e21e.ngbuilds.io/.

jelbourn

LGTM

Reviewed-for: public-api

jelbourn · 2020-12-09T17:00:06Z

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

+                    // 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)).


Might be useful to link to #39935 in this comment

jelbourn · 2020-12-09T17:00:06Z

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

+  // Clean the jqLite/jQuery data on the element and all its descendants.
+  // Equivalent to how jqLite/jQuery invoke `cleanData()` on an Element when removed:


nit: could probably use a JsDoc description on this function and on destroy app (which IDEs will pick up)

atscott

Reviewed-for: public-api

atscott · 2020-12-09T17:06:21Z

presubmit

AndrewKushnir

Reviewed-for: public-api

petebacondarwin

Can you explain why this is the case?

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

petebacondarwin · 2020-12-10T11:53:47Z

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

+
+                    // 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


Suggested change

// other usecases where desposing of an Angular/AngularJS app is necessary (such

// other use-cases where disposing of an Angular/AngularJS app is necessary (such

Ooops 😅 Fixed.

petebacondarwin · 2020-12-10T11:53:47Z

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

@@ -86,7 +86,7 @@ withEachNg1Version(() => {
           });
         }));

-      it('supports the compilerOptions argument', waitForAsync(() => {
+      it('should support the compilerOptions argument', waitForAsync(() => {


petebacondarwin · 2020-12-10T11:53:47Z

packages/upgrade/static/src/downgrade_module.ts

@@ -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


Suggested change

// other usecases where desposing of an Angular/AngularJS app is necessary (such

// other use-cases where disposing of an Angular/AngularJS app is necessary (such

Ooops 😅 Fixed.

This commit removes a couple of unused variables.

This commit moves the code for cleaning jqLite/jQuery data on an element to a re-usable helper function. This way it is easier to keep the code consistent across all places where we need to clean data (now and in the future).

…data

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 angular#39935

gkalpak · 2020-12-10T17:22:03Z

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

Can you explain why this is the case?

Expanded on this in the commit message. (Also updated the PR description.)

Thx for the reviews. I have addressed the comments. Marking for merging...

mary-poppins · 2020-12-10T17:25:20Z

You can preview 885710a at https://pr40045-885710a.ngbuilds.io/.

This commit removes a couple of unused variables. PR Close #40045

…40045) This commit moves the code for cleaning jqLite/jQuery data on an element to a re-usable helper function. This way it is easier to keep the code consistent across all places where we need to clean data (now and in the future). PR Close #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

…40045) This commit moves the code for cleaning jqLite/jQuery data on an element to a re-usable helper function. This way it is easier to keep the code consistent across all places where we need to clean data (now and in the future). PR Close #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

This commit removes a couple of unused variables. PR Close angular#40045

…ngular#40045) This commit moves the code for cleaning jqLite/jQuery data on an element to a re-usable helper function. This way it is easier to keep the code consistent across all places where we need to clean data (now and in the future). PR Close angular#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 angular#39935 PR Close angular#40045

This commit removes a couple of unused variables. PR Close angular#40045

…ngular#40045) This commit moves the code for cleaning jqLite/jQuery data on an element to a re-usable helper function. This way it is easier to keep the code consistent across all places where we need to clean data (now and in the future). PR Close angular#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 angular#39935 PR Close angular#40045

angular-automatic-lock-bot · 2021-01-11T16:57:23Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

google-cla bot added the cla: yes label Dec 9, 2020

gkalpak added comp: upgrade target: patch type: bug/fix labels Dec 9, 2020

ngbot bot added this to the Backlog milestone Dec 9, 2020

gkalpak marked this pull request as ready for review Dec 9, 2020

gkalpak added the action: merge label Dec 9, 2020

pullapprove bot requested review from alxhub, AndrewKushnir, atscott and jelbourn Dec 9, 2020

gkalpak requested a review from petebacondarwin Dec 9, 2020

jelbourn approved these changes Dec 9, 2020

View changes

atscott approved these changes Dec 9, 2020

View changes

pullapprove bot requested a review from IgorMinar Dec 9, 2020

pullapprove bot removed the comp: upgrade label Dec 9, 2020

ngbot bot removed this from the Backlog milestone Dec 9, 2020

AndrewKushnir approved these changes Dec 9, 2020

View changes

AndrewKushnir added the action: presubmit label Dec 9, 2020

mhevery added the comp: upgrade label Dec 9, 2020

ngbot bot added this to the Backlog milestone Dec 9, 2020

pullapprove bot removed the comp: upgrade label Dec 9, 2020

ngbot bot removed this from the Backlog milestone Dec 9, 2020

gkalpak added action: cleanup and removed action: merge labels Dec 9, 2020

petebacondarwin approved these changes Dec 10, 2020

View changes

gkalpak removed request for IgorMinar and alxhub Dec 10, 2020

gkalpak added 5 commits Dec 10, 2020

refactor(upgrade): remove unused variables

c1f5f66

This commit removes a couple of unused variables.

fixup! refactor(upgrade): create a helper for cleaning jqLite/jQuery …

e80c8f5

…data

fixup! fix(upgrade): fix HMR for hybrid applications

885710a

gkalpak force-pushed the fix-upgrade-hmr branch from 204e21e to 885710a Dec 10, 2020

gkalpak added action: merge and removed action: cleanup labels Dec 10, 2020

alxhub added a commit that referenced this issue Dec 10, 2020

refactor(upgrade): remove unused variables (#40045)

9a28ddf

This commit removes a couple of unused variables. PR Close #40045

alxhub closed this in 61376d5 Dec 10, 2020

gkalpak deleted the fix-upgrade-hmr branch Dec 11, 2020

zarend added a commit to zarend/angular that referenced this issue Dec 11, 2020

refactor(upgrade): remove unused variables (angular#40045)

fd573ad

This commit removes a couple of unused variables. PR Close angular#40045

twerske added a commit to twerske/angular that referenced this issue Jan 5, 2021

refactor(upgrade): remove unused variables (angular#40045)

d54caac

This commit removes a couple of unused variables. PR Close angular#40045

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 11, 2021

angular / angular Public

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

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

gkalpak commented Dec 9, 2020 •

edited

mary-poppins commented Dec 9, 2020

jelbourn left a comment

jelbourn Dec 9, 2020

gkalpak Dec 10, 2020

jelbourn Dec 9, 2020

gkalpak Dec 10, 2020

atscott left a comment

atscott commented Dec 9, 2020

AndrewKushnir left a comment

petebacondarwin left a comment

petebacondarwin Dec 10, 2020

gkalpak Dec 10, 2020

petebacondarwin Dec 10, 2020

petebacondarwin Dec 10, 2020

gkalpak Dec 10, 2020

gkalpak commented Dec 10, 2020

mary-poppins commented Dec 10, 2020

angular-automatic-lock-bot bot commented Jan 11, 2021

		// Clean the jqLite/jQuery data on the element and all its descendants.
		// Equivalent to how jqLite/jQuery invoke `cleanData()` on an Element when removed:

	// other usecases where desposing of an Angular/AngularJS app is necessary (such
	// other use-cases where disposing of an Angular/AngularJS app is necessary (such

angular / angular Public

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

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

Conversation

gkalpak commented Dec 9, 2020 • edited

mary-poppins commented Dec 9, 2020

jelbourn left a comment

atscott left a comment

atscott commented Dec 9, 2020

AndrewKushnir left a comment

petebacondarwin left a comment

gkalpak commented Dec 10, 2020

mary-poppins commented Dec 10, 2020

angular-automatic-lock-bot bot commented Jan 11, 2021

gkalpak commented Dec 9, 2020 •

edited