Skip to content

HMR - Services bootstrapped multiple times, but never torn down #1605

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

Open
1 task done
NickIliev opened this issue Nov 13, 2018 · 11 comments
Open
1 task done

HMR - Services bootstrapped multiple times, but never torn down #1605

NickIliev opened this issue Nov 13, 2018 · 11 comments

Comments

@NickIliev
Copy link

@larssn commented on Fri Nov 09 2018

Environment

  • CLI: 5.0.0

  • Cross-platform modules:
    ✔ Component nativescript has 5.0.0 version and is up to date.
    ✔ Component tns-core-modules has 5.0.2 version and is up to date.
    ✔ Component tns-android has 5.0.0 version and is up to date.
    ✔ Component tns-ios has 5.0.0 version and is up to date.

  • Android Runtime: 5.0.0

  • iOS Runtime: 5.0.0

  • Plugin(s): None

  • Node.js: 10.12.0

  • Please, attach your package.json and webpack.config.js as these configurations are usually critical for investigating issues with webpack.

These are default out of the box configs from using tns create

Describe the bug
When HMR tears down the Angular platform on changes, it correctly calls ngOnDestroy for all components etc, however it does not for services, meaning your app can end up in a weirdly initialized state with multiple "singletons" running side by side.

To Reproduce
tns create - create a new angular project with say, the tabbed view.
Insert a constructor in app/core/data.service.ts: constructor() { console.log("DataService constructor"); }
tns run android --hmr - the app starts, "DataService constructor" is written in console as expected.
Change some text in home.component.html that will cause a refresh. "DataService constructor" gets output again.

Expected behavior
ngOnDestroy is a valid lifecycle hook for services as well, as mentioned here: https://angular.io/api/core/OnDestroy

A lifecycle hook that is called when a directive, pipe, or service is destroyed.

You can imagine the trouble you can get in when you start including Rx Observable subscriptions in the constructor of a service, and even more when services start bootstrapping plugins.

So ngOnDestroy should also get called for services.

@NL33
Copy link

NL33 commented Oct 2, 2019

@NickIliev Can you please specify the solution to this? I am having a similar issue:

My app gets bootstrapped according to the number of times Hot Module Replacement has run. I see this bc I have an (i) applicationOn(suspendEvent... and (ii) applicationOn(resumeEvent)... in my app.module.ts.

These events fire the number of times that I have saved changes in the app, so they fire more the more hot module replacements I run.

This did not happen until I tried to do hot module replacement by adding "useLegacyWorkflow": false to nsconfig.json. to get HMR to run, I also stopped calling --env.uglify --env.aot in $ tns run.

Since then, this problem of multiplying bootstraps began.

Using NS 5.3.1, Angular.

Is there some kind of ngOnDestroy I need to call? What should happen in the destroy action?

@edusperoni
Copy link
Collaborator

Actually, this issue can be closed. ngOnDestroy is being called on services:

Refreshing application on device emulator-5554...
JS: onDestroy
JS: construct
Successfully synced application org.nativescript.comunicacaons on device emulator-5554.

@NL33 you have to call applicationOff on your ngOnDestroy. Here's what happening:

When you construct your service, you're subscribing to events: application.on(event => this.doStuff()). When you HMR, it calls ngOnDestroy and then creates new components and services that also subscribe to the same events.

Now you have 2 services (one in use and the other not being referenced anywhere) that call their respectives this.doSutff().

This is my current approach (handling back button on android):

@Injectable({
    providedIn: "root"
})
export class BackButtonService implements OnDestroy {
    constructor() {
        if (isAndroid) {
            app.android.on(app.AndroidApplication.activityBackPressedEvent,
                this.handleBackButton);
        }
    }

    ngOnDestroy() {
        if (isAndroid) {
            app.android.off(app.AndroidApplication.activityBackPressedEvent, this.handleBackButton);
        }
    }

// THIS IS IMPORTANT! don't use handleBackButton(..) {} because you'll lose this reference!
    handleBackButton = (data: app.AndroidActivityBackPressedEventData) => {
        // my code here
    }

}

@NL33
Copy link

NL33 commented Oct 4, 2019

Thanks very much, @edusperoni. Great info. To clarify, I am currently calling suspend and resume in my app.module.ts file (that is the main spot I put those actions--I assume that is not an issue), not a service. Unless app.module.ts is actually a service.

Using NS 5.3.1, Angular, focused on iOS.

So, for example, here is my code:

app.module.ts:

import { ios, resumeEvent, suspendEvent, on as applicationOn } from "tns-core-modules/application";

@NgModule({
    bootstrap: [
        AppComponent
    ],
    imports: [
        NativeScriptModule,
        AppRoutingModule
    ],
    declarations: [
        AppComponent
    ],
    schemas: [
        NO_ERRORS_SCHEMA
    ],
})

export class AppModule {

    public suspendEvent: any;
    public resumeEvent: any;
    public suspendEventEmit = new EventEmitter<any>()
    public resumeEventEmit = new EventEmitter<any>()

    constructor() {
      this.suspendEvent =  applicationOn(suspendEvent, (args) => {
            console.log('app is suspended')
            this.suspendEventEmit.emit('this app has been suspended')
      })

       this.resumeEvent = applicationOn(resumeEvent, (args) => {
            console.log('app is resumed)
            this.resumeEventEmit.emit('this app has been resumed')
       })
    }
}

And, as discussed, with Hot Module Replacement, the suspend and resume events get called an increasing amount depending on how many times HMR has been activated (ie, how many times I have made changes, and saved those changes).

So to avoid this problem, it seems like I should call ngOnDestroy() in app.module.ts. Therefore, I assume you mean I should add to app.module.ts:

import { off as applicationOff } from "tns-core-modules/application";
...

ngOnDestroy() {
    applicationOff(this.suspendEvent);
    applicationOff(this.resumeEvent);
}

I'm not familiar with the app.android.off... syntax you are using. Does the above code represent what you would have in mind?

Is this just necessary if I am using HMR? (if so, for production, I might consider removing HMR and this ngOnDestroy).

Related Note: I am having a possibly related issue with HMR: I have found that HMR sometimes hangs (stopping on "Webpack build done!"), and I think this happens if there is an error in my code. In the case of code error, seems like reloading is paused--which is a problem because it doesn't tell me what/where the error is!

@edusperoni
Copy link
Collaborator

@NL33 You shouldn't write stuff in your AppModule constructor!

Instead, use AppComponent and the ngOnInit and ngOnDestroy hooks (or even better, move it to a service and use Subjects).

It should look like this:

@Injectable({
    providedIn: "root"
})
export class ResumeSuspendService implements OnDestroy {
    public suspendEventEmit = new Subject<any>();
    public resumeEventEmit = new Subject<any>();

    constructor() {
      applicationOn(suspendEvent, this.suspendEvent)

      applicationOn(resumeEvent, this.resumeEvent)
    }

    ngOnDestroy() {
      applicationOff(this.suspendEvent);
      applicationOff(this.resumeEvent);
    }
    suspendEvent = (args) => {
            console.log('app is suspended')
            this.suspendEventEmit.next('this app has been suspended')
      };
    resumeEvent = (args) => {
            console.log('app is resumed)
            this.resumeEventEmit.next('this app has been resumed')
       };
}

You can then use it in your components:

export class MyComponent {
    constructor(private resumeSuspend: ResumeSuspendService) {}
    // you can now subscribe/unsubscribe to resumeSuspend.suspendEventEmit / resumeEventEmit in your ngOnInit/ngOnDestroy or use async pipes
}

@NL33
Copy link

NL33 commented Oct 4, 2019

Interesting. Thanks. I'll give it a shot.

On app.module: One of the reasons I am using app.module instead of a service is there are other things that I am monitoring in there. For example, Apple's universal links--I need to know when a universal link has been tapped to open the app. I'm using app.ios.delegate and applicationContinueUserActivityRestorationHandler, and I don't think that works outside of app.module.

So I have to: 1) register the universal link tap event, 2) get the details of the link, and then 3) send it off to the rest of the app. I don't think I can do (1) and (2) if I am not in app.module, but maybe app.component can work.

BTW, I have seen other things too that I think require code do work in app.module. For example, the nativescript-purchase plugin, for in-app purchases. You have to initialize the in app purchases, and it only seems to work if the code is in app.module.

@edusperoni
Copy link
Collaborator

You don't need to call them on the app module constructor. You can call them simply like this:

mystuff.init();

@NgModule({
    bootstrap: [
        AppComponent
    ],
    imports: [
        NativeScriptModule,
        AppRoutingModule
    ],
    declarations: [
        AppComponent
    ],
    schemas: [
        NO_ERRORS_SCHEMA
    ],
})
export class AppModule {
}

This way your code runs BEFORE angular is initialized. You can then set a global variable with some data which you then read when you're constructing your service (this is how most plugins are implemented). Running in your AppModule constructor is usually not recommended. If you REALLY need to do it, it's better to create a Service that you inject in your AppModule constructor or use APP_INITIALIZER (https://www.tektutorialshub.com/angular/angular-how-to-use-app-initializer/)

So you could have:

export class AppModule {
    constructor(private service: MyImportantService) {}
}

And it would actually initialize MyImportantService before constructing AppModule even

@NL33
Copy link

NL33 commented Oct 4, 2019

Sorry if I was unclear. The init of purchases for nativescript-purchase that happens in app.module does not happen in the constructor. Neither does the app delegate action to read the universal link. Right now, both of them happen in app.module above NgModule, like in your code.

So I assume it is fine to leave those in app.module, so long as they are above the constructor. I think what you are saying is:

  1. I can leave them in app module above the constructor, but
  2. take the other actions--like the resume and suspend event--and move them to a service. For example, the app delegate action in app module set a global variable equal to the universal link value, and then access that global variable in a service.

If that is correct, do you know how to set a global variable in code that appears before @NgModule?

for example, if this is above @NgModule in app.module:

app.ios.delegate = UIResponder.extend({
 applicationContinueUserActivityRestorationHandler: function(application, userActivity) {
            if (userActivity.activityType === NSUserActivityTypeBrowsingWeb) {
                tappedUrl = userActivity.webpageURL  **//HOW do I send tappedUrl into a global variable that other services can access?**

      }
            return true;
 }
})

The question is, as said above, what's the code to send 'tappedUrl' into a global variable that a service can access?

@edusperoni
Copy link
Collaborator

edusperoni commented Oct 4, 2019

If you can contain it into a service you should probably do so (application events usually fall into this). Otherwise, you can do the following:

// tapped-url.ts
export let tappedUrl = null;

export function init() {
  app.ios.delegate = UIResponder.extend({
   applicationContinueUserActivityRestorationHandler: function(application, userActivity) {
              if (userActivity.activityType === NSUserActivityTypeBrowsingWeb) {
                  tappedUrl = userActivity.webpageURL  **//HOW do I send tappedUrl into a global variable that other services can access?**

        }
              return true;
   }
  })
}

// app.module.ts

import { init } from './tapped-url';

init();

// myservice.ts or even CanActivate on your main route
import {tappedUrl} from './tapped-url';

// use tappedUrl

If you have events that you HAVE to subscribe at app init, you can store all events in an array (const eventQueue = [];) and then dispatch them when a function is called (MyService calls registerHandler(handlerHere) and you dispatch everything in eventQueue to that function). This is done by nativescript-firebase-plugin (https://github.com/EddyVerbruggen/nativescript-plugin-firebase/blob/master/src/messaging/messaging.ios.ts#L482) and nativescript-local-notifications (https://github.com/EddyVerbruggen/nativescript-local-notifications/blob/master/src/local-notifications.ios.ts#L342)

Basically, if you're dealing with stuff outside of angular, so you have to store them somewhere until angular is ready to receive it.

Edit: and it's important to always make angular components/services/etc self-contained, so make sure to clean-up angular stuff inside angular (for example, if you add a handler to something non-angular, remember to remove this reference on ngOnDestroy)

@NL33
Copy link

NL33 commented Oct 4, 2019

Thanks. Your code works for me. I was missing wrapping the relevant function in an export function, and then calling that function in app.module.

So this should all be working now (haven't had time to fully test, but I expect it works). For future reference, do you know what's so bad about having functions in app module, such as having the onsuspend and onresume events?

@edusperoni
Copy link
Collaborator

Although Modules seem to have onDestroy hooks, how would you pass these events down to components? You'd need a service for that. The AppModule code would also become very bloated.

You'd expect your modules and services to be self-contained, so it's bad design to write stuff in your AppModule and have a bunch of code that depend on it on child modules (you have made the dependency arrows go both ways, now your parent depends on children and your children depend on the parent).

I wrote a simple example on stackblitz that is a good starting point for anyone trying to do the same:
https://stackblitz.com/edit/angular-native-events

@NL33
Copy link

NL33 commented Oct 5, 2019

Awesome. You've helped me out a lot here. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants