Ionic-framework: Overlays in 2.0.0-beta.11

[jimitndiaye]

Short description of the problem:

After upgrading to beta.11 (which I had to do because it finally brings support for @angular/forms) and applying the code changes suggested in the upgrade guide (use AlertController.create, and alert.present rather than NavController.present) my app is broken. I think this is because of the decision to make overlays global (app-wide) rather than constrained by the containing NavController.

Running the following code in my app results in an error (the same code used to work in beta.10 using nav.present):

const alert = this.alertController.create({/*some options*/);
alert.present().

The above code results in the following error:

TypeError: Cannot read property 'insertViews' of undefined
    at App.present (eval at <anonymous> (main.bundle.js:1), <anonymous>:171:28)
    at Alert.present (eval at <anonymous> (main.bundle.js:1), <anonymous>:98:26)
    at DialogExplorerComponent.addNode (eval at 1473 (1.chunk.js:1), <anonymous>:138:16)

Stepping through the code in the debug console reveals that the _portal field of that App instance is undefined:
image

Is there some way of using the beta.11 API to achieve the same behavior as beta.10, that is, having overlays be constrained by their containing NavController rather than app-wide as in beta 11?

What behavior are you expecting?

That I'd be able to use overlays like Alert, Toast as I could in beta.10.

Which Ionic Version? 1.x or 2.x
2.0.0-beta.11

[jgw96]

Hello, thanks for opening an issue with us! Would you be able to provide a repo i can use to reproduce this issue? Thanks for using Ionic!

[Kobzol]

Could you please modify the New issue template to include a ionic-angular beta 11 plnkr? I also have some issues with the overlays, but I can't create the plnker example, as it cannot find @angular/forms on npmcdn.

[jimitndiaye]

@jgw96 I'll see what i can do to provide a minimal repo but it may be tricky as I'm not 100% sure what the root cause is so may trouble isolating it from our codebase.
I can only assume it's something to do with overlays now being global rather than scoped. This part of our app has multiple separate root navs rather than a single one.

[jgw96]

Hello @Kobzol and @jimitndiaye . Unfortunately their is an issue with npmcdn and the @angular/forms module that I am working on trying to solve, but it may be a little while before it is working again. In the meantime if you could comment with a repo or code example that i can use to repro that would be great! Also, i realize this isnt ideal, and am sorry for that. @jimitndiaye If you dont mind me asking what is the use case behind having multiple separate root navs in your app?

[jimitndiaye]

@jgw96 The particular page is for desktop only but uses the some of the same Ionic controls for consistency with the more mobile-focused parts of the app (it's a PWA-esque app). The reason for multiple navs is that we needed independent navigation within separate sections of the page.

[TheMadBug]

Not sure if this is due to the same root cause, but for me I've found DI has broken for my popups.
Upgraded to Beta 11 and changed how Popup's get created to be via the PopupController with constructor injection.
A vanilla popup works fine, but if the component that we're passing into the popup relies on a custom service that we put in the root component's providers, it can't seem to find it where previously it did. So just end up getting a "No provider for XXXX".

[szerner]

@TheMadBug Please look at this issue, the solution might be to put your service as a provider into your ionicBootstrap() ('app.ts').

[jimitndiaye]

I was able to rule out the multiple navcontrollers as being the cause by wrapping the entire app temporarily in a dummy component with a nav controller resulting in one root nav. Still getting the same error : "Cannot read property 'insertViews' of undefined".

[jimitndiaye]

@szerner @TheMadBug I'm wondering if issue #7592 might not be the ultimate culprit here. Obviously there's been a change in beta 11 affecting DI and I'm wondering if the injectors used on portals/overlays/navs etc in beta 11 are properly reusing their parent injectors. It could explain why we're getting null reference errors (we could be getting a new instance of App instead of the preconfigured global instance resulting in an undefined _portal because setPortal was never called on that instance). Injector inheritance should follow the ng2 model and registering providers in ionicBootstrap alone is not a solution for that.

[jgw96]

Hello @jimitndiaye ! Thanks for all the testing, this is very interesting. Is their any way that you could post a little example that i can use to reproduce this issue? I cant seem to reproduce the issue right now, but i would love to see what your code looks like.

[jimitndiaye]

Hi @jgw96,
I've created a minimal repository based on angular2-webpack-starter that most closely mirrors our app and added an ionic page with a button that attempts to display an alert and was able to reproduce the error.

[jgw96]

Thanks @jimitndiaye ! So i can definitely reproduce the issue in your demo project, but currently trying to figure out why i cant reproduce it in any of our tests or in any of my test apps. If you get a chance, could you try with a fresh starter project (ionic start test-app --v2) and see if you can reproduce their? Thanks for using Ionic!

[jimitndiaye]

@jgw96 I'll give it a shot but the ionic templates are sufficiently different from the ng2-webpack-starter that it may prove non-trivial.

[jimitndiaye]

@jgw96 the key difference may be hosting the '''ion-nav''' in a component loaded within a '''router-outlet'''

[jgw96]

Quick question: Are you mixing Ionic in a regular Angular 2 app in your app your having the issue with?

[jimitndiaye]

@jgw96 the app is basically identical to the repository I linked above.

[jgw96]

@jimitndiaye ahh, this may very well be the issue then. Ionic 2 is not currently built to work with the Angular router and since your displaying the ionic part of your app in a router-outlet it is going to cause issues.

[jgw96]

Alright so i reviewed with my team mates and we have come to a conclusion. At this time we cannot recommend mixing an Ionic 2 app with an Angular 2 app. Their is a long list of reasons for this but the short story is:

1. Ionic 2 is not meant to be used with the Angular router at this time. While this may have worked in the past, with the current changes we are making (and future changes we are working on) this is not going to be a supported way to use Ionic 2.
2. By using Angular 2 with Ionic 2 you are going to run into issues with providers and directives not bootstrapping correctly as our custom bootstrapping process works a bit differently than a normal Angular 2 bootstrapping process.
3. Using Ionic 2 inside an Angular app is not currently a supported use case. Ionic 2 is meant to be used to build Ionic 2 apps and not mix with plain Angular 2.

Now, just to be clear, their is an issue currently with providers that is being looked into #7160 that may be related to this issue, but the core if this issue is using Ionic 2 inside Angular 2. If you would like to get our mobile oriented styles you can just grab our css and use that in your Angular 2 app if you would like, but that is not officially recommended. Because of all of the above I am going to be closing this issue.

ps: Is their a reason that you are not building your app purely with Ionic 2? If so, we would love to know so that we can incorporate that feedback into Ionic!

[jimitndiaye]

@jgw96 I can't say I'm not disappointed by that conclusion. In my opinion if Ionic2 runs on ng2 it should be a good citizen and support basic ng2 scenarios. As you said, this used to work in beta 10 and was broken in beta 11. Looking at the issue list it seems that a lot of scenarios were broken by the recent changes to overlays, including ours, suggesting that maybe the decisions made for beta 11 may need review.
The main reason we're not using ionic navigation for all parts of the app is support for asynchronous loading of sections of the app a la PWAs. Some optional parts of the app are to big to load when not needed. We use the router to load those bits and then it's ionic from there on out.

[jgw96]

Hello @jimitndiaye ! Thanks for replying and for your honesty! I will try to explain this conclusion as best as i can. So, even though Ionic 2 is built with Angular 2 there are fundamental differences. Also Angular 2 and Ionic 2 have slightly different use cases. Ionic is mainly focused on the mobile world where navigation and routing are very different than desktop, while Angular 2 is more of a general purpose framework (as it should be), therefore they are not really meant to be used together. Also, one of the main reasons we changed the way overlay components now work is to match up better with how native sdk's work, and while this may have broken some use cases (and we are currently looking into those to consider them), we are in beta which means that their will be certain changes. Also, the main issue with overlay components currently is centered around a providers issue, which is currently being worked on. Finally, their are some very cool changes coming soon to beta.12 which will cover the routing use case you describe above, which should mean that you will be able to change your app to be built purely with Ionic 2. I completely understand your disappointment, but i hope that my explanation helps you understand our conclusion.

[jimitndiaye]

@jgw96 thanks for that in depth response. I look forward to the changes coming in beta 12. The one thing I'd like to add is that if the intent is to support progressive web apps then the desktop scenario should be considered when deciding the navigation/routing infrastructure since they are the gateway to users deciding to install the app (in addition to direct download from an app store)

[jgw96]

@jimitndiaye No problem! Also i definitely agree with you about the desktop scenario. The biggest difference currently between us and angular is the routing stuff (which is needed for pwa's) but we are working on a few things that should solve all this (:

[jimitndiaye]

@jgw96 FYI: I tested stripping out the router by directly loading the ion-nav and you still get the same error. I don't think it is related to use of the router. I updated the repo to illustrate it. It's basically just an AppComponent with an ion-nav that loads a page with a button.

/*
 * App Component
 * Top Level Component
 */
@Component({
  selector: 'app',
  encapsulation: ViewEncapsulation.None,
  styleUrls: [
    './app.style.css'
  ],
  template: `
      <ion-nav [root]="navRoot"></ion-nav>
  `
})
export class App {
  navRoot = IonicPage;

}

and the loaded page:

@Component({
  selector: 'ionic-page',

  template: `
  <ion-header>
    <ion-toolbar>
      <ion-title>Ionic Page</ion-title>
    </ion-toolbar>
  </ion-header>
  <ion-content>
    <h1>Hello from Ionic Page</h1>
    <button (click)="showModal()">Show Alert</button>
  </ion-content>
  `
})
export class IonicPage {
  constructor(private alertController: AlertController) {

  }

  ngOnInit() {
    console.log('hello `IonicPage` component');
  }

  showModal() {
    const alert = this.alertController.create({
      buttons: ['Ok'],
      message: "Here's an alert",
      title: 'My alert'
    });
    alert.present();
  }
}

You still get the error.

[TheBrockEllis]

Hi all!

I ran into the same issue today working with a popover in beta.11. DI seems to be wonky. It won't find the services I registered as providers with the App component in the popover.

I was able to reproduce this with a blank v2 template. You can see the test repository here: Ionic2-Popover-DI-Test.

• ionic start ionic2-popover-di-test --v2
• mkdir providers && touch providers/thing.ts
• Made the ThingService have some member property and one method that just returns an array of dummy data
• Added ThingService as a global provider by adding it to the providers: [] metadata in the App/Component decorator
• Added a button to the HomePage and imported the PopoverController into the HomePage and insert into constructor.
• Create PopoverPage that imports the ThingService and injects it into the constructor
• Method added to HomePage that creates a new popover using PopoverPage

Sorry if any of my terminology is off. Still new to this rodeo. =)

[jgw96]

Hey @TheBrockEllis thanks for the info! So this sounds like the same issue that were tracking here. The fix for this should be in beta.12, and will probably make it into one of the nightlies before then. When that fix hits i will definitely let you know so you see if that fixes it for you!