ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.02k stars 13.51k forks source link

ionic4/angular: overlay `getTop` can return `undefined` #17801

Closed Pierrci closed 5 years ago

Pierrci commented 5 years ago

Bug Report

Ionic version:

[x] 4.x

Current behavior: In the angular package, the return type of the getTop method in OverlayBaseController is Promise<Overlay>

Expected behavior: The return type should be Promise<Overlay | undefined>

Steps to reproduce: Use any component extending OverlayBaseController and call the getTop method without any alert currently present. With the AlertController for example, returned object will be of type Promise<HTMLIonAlertElement>, while it should be Promise<HTMLIonAlertElement | undefined>.

Related code:

import { Component, OnInit } from '@angular/core';
import { AlertController } from '@ionic/angular';

@Component({
  selector: 'app-home',
  templateUrl: 'home.page.html',
  styleUrls: ['home.page.scss'],
})
export class HomePage implements OnInit {
  constructor(private alertController: AlertController) {}

  async ngOnInit() {
    const undefinedAlert = await this.alertController.getTop(); // typed as HTMLIonAlertElement
    alert(`type of alert is ${typeof undefinedAlert}`); // typeof undefinedAlert === 'undefined'
    undefinedAlert.dismiss(); // still possible because of the typing, will throw an error
  }
}

Other information:

Ionic info:

Ionic:

   ionic (Ionic CLI)             : 4.10.3 (/usr/local/lib/node_modules/ionic)
   Ionic Framework               : @ionic/angular 4.1.1
   @angular-devkit/build-angular : 0.13.6
   @angular-devkit/schematics    : 7.2.4
   @angular/cli                  : 7.3.6
   @ionic/angular-toolkit        : 1.4.0

System:

   NodeJS : v11.6.0 (/usr/local/bin/node)
   npm    : 6.5.0
   OS     : macOS Mojave
Pierrci commented 5 years ago

Just submitted the related PR

brandyscarney commented 5 years ago

It seems the PR was merged so I'm going to close this, thanks for the issue!

ionitron-bot[bot] commented 5 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.