fix(ngUpgrade): example bootstraps in the wrong zone

[wardbell]

@mhevery sez:

I just noticed that there is a wrong ngUpgrade example in our docs. Could we get it fixed?

The issue is here:

import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';
platformBrowserDynamic().bootstrapModule(AppModule).then(platformRef => {
  const upgrade = platformRef.injector.get(UpgradeModule) as UpgradeModule;
  upgrade.bootstrap(document.body, ['heroApp'], {strictDi: true});
});

Because .then gets invoked in the same zone as the zone at the time of invocation it means that it will run in the <root> zone. The implication is that when upgrade.bootstrap executes it too will run in <root> zone. This creates subtle problems.

In my opinion the fix is to move the upgrade.bootstrap call into the constructor of the the top level module. The constructor is invoked in the correct NgZone.

import { HeroDetailComponent } from './hero-detail.component';
@NgModule({
  imports: [
    BrowserModule,
    UpgradeModule
  ]
})
export class AppModule {
  constructor(upgrade: UpgradeModule) {
    upgrade.bootstrap(document.body, ['heroApp'], {strictDi: true});
  }

  ngDoBootstrap() {}
}

[wardbell]

@petebacondarwin weighed in

Hi Miško, Can you elaborate on the bugs that you are describing? Have you tested that moving it actually fixes these subtle bugs? What about moving it to the ngDoBootstrap hook?

[mhevery]

calling upgrade.bootstrap means that ng1 bootstraps in the wrong zone. For the most part that is not an issue unless there is an eager service in the ng1 bootstrap which pools in ng2 service, which in turn does setTimeout or addEventListener. In that case the async operation is done in the wrong zone. Since most simple hello world examples do not create such scenarios, this only comes into play with more complex apps, which in turn are harder to debug.

The other thing which happens is that people put additional initialization code in the then which also runs the risk of doing wrong outside of angular zone.

The correct thing to do is to always bootstrap in the NgZone.

[mhevery]

@petebacondarwin BTW, at the very least we should throw when upgrade.bootstrap is called and it is not in ng2zone.

[petebacondarwin]

If upgrade.bootstrap should run inside the NgZone then we should just make sure it does so by changing that method. Moving the call of the method elsewhere seems like a workaround, no?

[mhevery]

1. yes we should make sure that upgrade.bootstrap is in zone, but that only fixes half the problem. People often boot their app initialization code in the then which than suffers the same fate.

2. What is your concern about saying that the best practice is to boostrap from the module?

On Tue, Feb 28, 2017 at 10:22 PM, Pete Bacon Darwin < [email protected]> wrote:

If upgrade.bootstrap should run inside the NgZone then we should just make sure it does so by changing that method. Moving the call of the method elsewhere seems like a workaround, no?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/angular/angular.io/issues/3317#issuecomment-283255701, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG1Tx97AYLLFJIxuJwxxVTWC3CAW-wiks5rhQ6XgaJpZM4MO2ER .

[petebacondarwin]

OK we have spoken and I understand the problem. The first port of call is to update the docs and explain why it was wrong. We are going to change the library to throw an error if they do it the then way. I will send in a PR tomorrow for the docs.

[petebacondarwin]

I have a branch ready to fix this but I was holding off because I didn't have a good solution for the dual router situation. If we agree to remove that from the guide, then we can merge my branch.

[jeffeld]

Good to hear this is sorted. Just spent 2 days going round in circles over this.

http://stackoverflow.com/questions/43870336/injecting-angular-1-service-into-angular-4/43896756#43896756

[petebacondarwin]

See https://github.com/angular/angular/pull/16409

[jensbodal]

@petebacondarwin could you elaborate on the dual router situation?

I'm running a hybrid Angular/AngularJS application with ui-router handling router in AngularJS, but when I switch to bootstrapping AngularJS in the AppModule my AJS view is no longer working.

WORKING: https://plnkr.co/edit/36zstIqjEWU8uo9UXCc0

NOT WORKING: https://plnkr.co/edit/ehVikipem5sINhAnz3bp

---

EDIT

Well for what it's worth I can now get my non-working example to work using an idea found here. I move the ui-view logic to an angular.js component which then also includes the Angular app-root component: https://plnkr.co/edit/CR4UUAHRZxtzrEaFvUEt

Downgrading the AppComponent doesn't appear necessary since it is still listed in the bootstrap section.

This can be expanded on further to use a combination of router-outlet and ui-view to show angular/angularjs views. Is bootstrapping the app in this somewhat convoluted way a bad idea?

I have a demo app I'm working on that shows how I used this to show the different routes: https://github.com/jensbodal/ng1-ng2-hybrid/compare/branch-step-5...branch-step-5-alternative