fix(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent by m-abs · Pull Request #1728 · NativeScript/nativescript-angular · GitHub

The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/CONTRIBUTING.md#commit-messages.
There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it.
You have signed the CLA.
All existing tests are passing: https://github.com/NativeScript/nativescript-angular/blob/master/DevelopmentWorkflow.md#running-the-tests
Tests for the changes are included. Don't see how

What is the current behavior?

The application module isn't destroyed when the application exitEvent is triggered.
This causes the previous instance of the nativescript-angular app to be running in a sleep state and ngOnDestroy() on it's compoments, services etc. are not called as expected.

What is the new behavior?

Adds an event listener for the exitEvent in the NativeScriptPlatformRef which destroys the lastModuleRef which triggers ngOnDestroy() as expected.

Fixes #923

test

LGTM - triggered the CI.
Also I think this is more of a fix than a chore so I have renamed the PR title.

test

…escript-angular into fix/923-module-not-destroyed

@cla-bot check

The cla-bot has been summoned, and re-checked this pull request!

Hi @vakrilov,

I just noticed a problem

On Android 9.

Launch an app with this patch applied.
Go to system settings -> Accessibility -> Font size and change the font size.
Go back to the app.

This will have triggered an exit event and tried to destroy the angular application, but it is still running.
Now in a broken state.

You can see the problem here:

Checkout https://github.com/m-abs/tns-ng-leak
npm install && bash ./patches/apply-patches.sh
Follow the steps above.

I print out the app events:

JS: suspend # Leave the app to change the font size
JS: activityPaused
JS: activityStopped
JS: orientationChanged # Font size changed in while I'm in settings
JS: exit # Returned to the app
JS: activityDestroyed
JS: activityCreated
JS: activityStarted
JS: activityResumed
JS: resume
JS: displayed
JS: displayed

If exit event was triggered twice we would try to destroy an already destroy module ref.

…escript-angular into fix/923-module-not-destroyed

Don't destroy the angular module on restart

I solved the problem.

The exit event is triggered because the MainActivity.onDestroy() is called as a part of a restart needed to apply the new settings.

I added a test for this case in the exit event handler.

test

chor(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent

a21454f

ghost added the ♥ community PR label Feb 12, 2019

fix(tslint): missing whitespace

a4ee021

vakrilov previously approved these changes Feb 14, 2019

View reviewed changes

vakrilov changed the title chor(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent fix(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent Feb 14, 2019

This was referenced Feb 17, 2019

Potentiel leak: NativeScript views not cleaned up on removal #1738

Closed

Fix/ns views not cleaned on removal #1740

Merged

Merge branch 'master' into fix/923-module-not-destroyed

1119e2a

ghost assigned vakrilov Feb 19, 2019

ghost added in progress and removed ♥ community PR labels Feb 19, 2019

edusperoni mentioned this pull request Feb 19, 2019

Error: Uncaught (in promise): Firebase already initialized EddyVerbruggen/nativescript-plugin-firebase#944

Closed

m-abs added 3 commits February 20, 2019 15:28

Merge branch 'master' into fix/923-module-not-destroyed

bdd3250

Merge branch 'fix/923-module-not-destroyed' of github.com:m-abs/nativ…

f9b1d0c

…escript-angular into fix/923-module-not-destroyed

fix: remove rootContent on exit

14e787f

m-abs dismissed vakrilov’s stale review via 14e787f February 21, 2019 13:26

vakrilov previously approved these changes Feb 21, 2019

View reviewed changes

fix: page might be null'ed before clearHistory's navigatedToEvent

ea66985

m-abs dismissed vakrilov’s stale review via ea66985 February 22, 2019 10:40

cla-bot Bot added the cla: yes label Feb 25, 2019

Merge branch 'master' into fix/923-module-not-destroyed

16af2e8

m-abs added 3 commits March 8, 2019 15:40

fix: on destroy remove the lastBootstrappedModule

5e13263

If exit event was triggered twice we would try to destroy an already destroy module ref.

Merge branch 'fix/923-module-not-destroyed' of github.com:m-abs/nativ…

30db0d2

…escript-angular into fix/923-module-not-destroyed

fix: the exit event is triggered on restart

3dffbd5

Don't destroy the angular module on restart

m-abs and others added 2 commits March 12, 2019 14:27

Merge branch 'master' into fix/923-module-not-destroyed

9122c23

Merge branch 'master' into fix/923-module-not-destroyed

72d877b

vakrilov approved these changes Apr 5, 2019

View reviewed changes

Merge branch 'master' into fix/923-module-not-destroyed

ac9a59f

ghost assigned elena-p Apr 12, 2019

elena-p merged commit f12000e into NativeScript:master Apr 15, 2019

ghost removed the in progress label Apr 15, 2019

elena-p mentioned this pull request Apr 23, 2019

release: cut the 7.2.4 release #1790

Closed

fix(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent by m-abs · Pull Request #1728 · NativeScript/nativescript-angular · GitHub

Uh oh!

fix(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent - #1728

fix(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent#1728
elena-p merged 14 commits into
NativeScript:masterfrom
m-abs:fix/923-module-not-destroyed

m-abs commented Feb 12, 2019 •
edited
Loading

Uh oh!

Uh oh!

vakrilov commented Feb 14, 2019

Uh oh!

vakrilov commented Feb 14, 2019 •
edited
Loading

Uh oh!

Uh oh!

vakrilov commented Feb 19, 2019

Uh oh!

etabakov commented Feb 25, 2019

Uh oh!

cla-bot Bot commented Feb 25, 2019

Uh oh!

m-abs commented Mar 8, 2019 •
edited
Loading

Uh oh!

Uh oh!

m-abs commented Mar 11, 2019

Uh oh!

SvetoslavTsenov commented Apr 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fix(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent by m-abs · Pull Request #1728 · NativeScript/nativescript-angular · GitHub

Uh oh!

fix(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent - #1728

fix(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent#1728elena-p merged 14 commits intoNativeScript:masterNativeScript/nativescript-angular:masterfrom m-abs:fix/923-module-not-destroyedCopy head branch name to clipboard

Conversation

m-abs commented Feb 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the current behavior?

What is the new behavior?

Uh oh!

vakrilov commented Feb 14, 2019

Uh oh!

vakrilov commented Feb 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vakrilov commented Feb 19, 2019

Uh oh!

etabakov commented Feb 25, 2019

Uh oh!

cla-bot Bot commented Feb 25, 2019

Uh oh!

m-abs commented Mar 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m-abs commented Mar 11, 2019

Uh oh!

SvetoslavTsenov commented Apr 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fix(NativeScriptPlatformRef): Destroy lastModuleRef on exitEvent#1728
elena-p merged 14 commits into
NativeScript:masterfrom
m-abs:fix/923-module-not-destroyed

m-abs commented Feb 12, 2019 •
edited
Loading

vakrilov commented Feb 14, 2019 •
edited
Loading

m-abs commented Mar 8, 2019 •
edited
Loading