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

bug: #23432

Closed selected-pixel-jameson closed 3 years ago

selected-pixel-jameson commented 3 years ago

Bug Report

Ionic version: [x] 5.4.0

Angular Version 5.4.

Current behavior: I updated my iOS device to 14.6 and I'm noticing performance issues when navigating between pages. I've tested the same App on iOS 14.5 and I'm not seeing the same behavior. I have since updated the test device that had 14.5 on it to 14.6 and I'm now seeing the same performance issues.

Expected behavior: I wouldn't expect this to be an issue with a minor iOS update.

Steps to reproduce: Build out a UI workflow that navigates by pushing pages onto the angular navigation controller.

Other information: We are working on getting Ionic updated to the latest version and hopefully this will resolve these issues. There was an existing bug that was preventing us from updating past Ionic 5.4 that has since been resolved.

Ionic info: Ionic:

Ionic CLI : 6.16.1 (.nvm/versions/node/v12.22.1/lib/node_modules/@ionic/cli) Ionic Framework : @ionic/angular 5.4.0 @angular-devkit/build-angular : 0.900.6 @angular-devkit/schematics : 9.0.6 @angular/cli : 9.1.12 @ionic/angular-toolkit : 2.2.0

Capacitor:

Capacitor CLI : 2.4.4 @capacitor/android : 2.4.4 @capacitor/core : 2.4.4 @capacitor/ios : 2.4.4

Utility:

cordova-res : not installed globally native-run : not installed globally

System:

NodeJS : v12.22.1 (/nvm/versions/node/v12.22.1/bin/node) npm : 6.14.12 OS : macOS Big Sur

Is anyone else seeing issues with performance after updating to iOS 14.6 on Ionic Angular Apps?

liamdebeasi commented 3 years ago

Thanks for the issue. Can you reproduce the issue in an Ionic starter app and provide a link to the repo?

selected-pixel-jameson commented 3 years ago

I will sure try. I just wanted to see if other people were experiencing the issue first and try updating to the latest Ionic version. I’ll update the bug at that point.

Jameson Parker Owner & Software Engineer www.selectedpixel.com www.linkedin.com/in/jamesonwparker (920) 203-4106

On Jun 8, 2021, at 8:39 AM, Liam DeBeasi @.***> wrote:

Thanks for the issue. Can you reproduce the issue in an Ionic starter app and provide a link to the repo?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ionic-team/ionic-framework/issues/23432#issuecomment-856776784, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGXF36JELI6MMHHXFNM4RODTRYMSBANCNFSM46J6W6XQ.

AE1NS commented 3 years ago

Here is a ticket at the capacitor issues: https://github.com/ionic-team/capacitor/issues/4187

Maybe the reported performance issues with ionic/capacitor (in general) are related to the same issue.

tafelnl commented 3 years ago

@AE1NS I do not suspect these issues to be related. Your issue is specifically about Android performance. This issue is about performance on iOS.

selected-pixel-jameson commented 3 years ago

Update: I've updated to the latest version of Ionic ( 5.6.9 ) with Angular 10 and I'm still seeing this issue. This does not happen on iOS Devices running an iOS version prior to 14.6. This does not happen with Android devices either. Only iOS versions running 14.6.

Note that I've tested this on an iPhone 12 Pro and an iPhone 8 Plus so it's not a hardware issue.

https://user-images.githubusercontent.com/28204537/122591496-a591d800-d028-11eb-90e1-9fbc6d2c5e45.mov

liamdebeasi commented 3 years ago

Thanks for the follow up. Can you please reproduce this issue in an Ionic starter app and provide a link to the repo? It is going to be hard for me to identify what the issue may be since I cannot reproduce it on my end currently.

ionitron-bot[bot] commented 3 years ago

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

selected-pixel-jameson commented 3 years ago

Man.... Couldn't feel more brushed aside. 😞

This is a pretty huge issue for us and it's related to a minor iOS update and I've verified with numerous devices that this isn't an issue on iOS 14.5 and is now an issue on iOS 14.6.

Re-creating an app that involves this amount of functionality piece by piece to see where it fails is going to be pretty intensive. I understand that this is open source and free, but you kind of take it on to support these types of things when you push it out into the developer community.

Some kind of insight sure would be helpful as this bug is related to a platform update and not a code update on our end.

liamdebeasi commented 3 years ago

I don't mean for you to feel brushed aside, but without being able to reproduce the issue on my end it is going to be impossible for me to fix the issue. We ask for GitHub reproductions of the issue in order to better understand the issue.

If this is a problem with iOS 14.6 but not iOS 14.5, then this is likely a bug in WebKit not Ionic Framework. Are you able to send over the repo for the app in that video?

selected-pixel-jameson commented 3 years ago

I agree it has to be something with Webkit as this problem doesn't happen on Android devices. I'm updating my device to iOS 14.7 to see if that resolves the problem.

Unfortunately I cannot send the repo as this is a commercial project. Is there someone on the Ionic team that we could potential hire to help resolve this issue ASAP?

liamdebeasi commented 3 years ago

If you have Ionic Enterprise Support (https://ionic.io/enterprise) you can file a ticket through the appropriate channels, but otherwise we do not offer other paid support services.

Are you able to remove anything proprietary from the app experiencing this issue and send that over? (I.e. replace any secret text or images with lorem ipsum and placeholder images). That might be the path of least resistance here in terms of reproducing the bug and not sharing any proprietary code.

selected-pixel-jameson commented 3 years ago

Let me see if updating to iOS 14.7 solves the issue 🤞 then we'll go down that road. Thank you!

selected-pixel-jameson commented 3 years ago

I believe I have identified the issue. It seems to be related to having nested Ion-content. I'm trying to determine why I had to do this initially. I believe it had something to do with scrolling.

selected-pixel-jameson commented 3 years ago

Ugh.. nope that wasn't it. Just removed the ability to scroll.

liamdebeasi commented 3 years ago

One thing to check is if you are doing anything particularly expensive in the ionViewWillEnter hook of the entering page or the ionViewWillLeave hook of the leaving page. I've seen a few apps that try to fire off network requests or process large amounts of data that end up causing main thread jank.

selected-pixel-jameson commented 3 years ago

Issue is still there in iOS 14.7 :(

selected-pixel-jameson commented 3 years ago

Ok. Ultimately it seems like this is boiling down to me trying to render content while the navigation animation is still taking place.

The page that is being rendered is very complicated and does have lots of *ngIf statements in it. So I can try to optimize all of that.

But ultimately if I just wrap the code that is triggering the rendering to happen in a timeout so that it is executed after the page navigation completes there is no lag.

With that said.

  1. It would be ideal to have this content rendered when / during the navigation. This looks much better then having users go to a blank page and have it loading. If the content can be loaded it should be there when they navigate.
  2. It was working prior to iOS 14.6 fine. Now it's now.

I'm sure there is a ton of optimization that could be done on this page.

<ion-header>
  <ion-toolbar *ngIf="!hideFirstToolbar">
    <ion-buttons slot="start">
      <ion-back-button defaultHref="tabs/tab1/menu"></ion-back-button>
    </ion-buttons>
    <ion-title *ngIf="title" [ngClass]="{'small-title':url ? true:false}">{{title}}</ion-title>
    <ion-buttons slot="end">
      <ion-button *ngIf="content && content.url" (click)="openWebsite(content.url)">
        <ion-icon name="globe-outline"></ion-icon>
      </ion-button>
    </ion-buttons>
  </ion-toolbar>
  <ion-toolbar *ngIf="!hideSecondaryToolbar && !subscriptionRequired" class="force-no-padding force-no-margin">
    <div *ngIf="subtitle" class="fade-in sub-header">{{subtitle}}</div>
    <ng-container *ngIf="store && store.filterMenu">
      <ion-grid class="ion-no-padding ion-no-margin">
        <ion-row class="ion-no-padding ion-no-margin">
          <ion-col size="10" class="ion-no-padding ion-no-margin search-column">
            <ion-searchbar enterkeyhint="done" inputmode="search" showCancelButton="never" debounce="500" type="text"
              (ionChange)="handleSearchInput($event)" (keyup)="handleKeyPress($event)"
              placeholder="{{searchPlaceholder}}">
            </ion-searchbar>
          </ion-col>
          <ion-col size="2" class="ion-padding-horizontal ion-no-margin filter-button-column">
            <ion-button size="small" class="ion-text-end" *ngIf="store && store.filterMenu" id="search-menu-button"
              fill="clear" (click)="openSearchMenu(store.filterMenu)">
              <div *ngIf="settingsStore.activeFilterCount" class="active-indicator detail">
                {{settingsStore.activeFilterCount}}
              </div>
              <ion-icon size="large" color="dark" name="options-outline"></ion-icon>
            </ion-button>

          </ion-col>
        </ion-row>
      </ion-grid>
    </ng-container>
  </ion-toolbar>
</ion-header>
<ion-content scroll-y="false">
  <app-skeleton-list *ngIf="contentType && !content && !subscriptionRequired"></app-skeleton-list>
  <ng-container *ngIf="!subscriptionRequired">
    <!-- Show Category Issue Series Toggle -->
    <ng-container *ngIf="content && content.type === 'category' && content.seriesFirst">
      <ion-grid class="ion-no-margin ion-no-padding">
        <ion-row>
          <ion-col offset="1" size="10" class="ion-align-self-center">
            <ion-segment name="segment" class="fade-in" (ionChange)="viewChanged($event)" [(ngModel)]="viewSegment">
              <ion-segment-button value="series">
                <ion-label>Title View</ion-label>
              </ion-segment-button>
              <ion-segment-button value="issue">
                <ion-label>Recently Added</ion-label>
              </ion-segment-button>
            </ion-segment>
          </ion-col>
        </ion-row>
      </ion-grid>
    </ng-container>
    <!-- End Show Category Issue Series Toggle -->
    <ion-content class="search-scroll-content" (touchmove)="onScrollStart($event)">
      <div *ngIf="filterPadding" [ngClass]="{'filter-padding':filterPadding}"></div>
      <ng-container *ngIf="content && !hideContent">
        <app-category-details *ngIf="content.type == 'category' && !params['search']" [content]="content">
        </app-category-details>
        <app-category-details *ngIf="content.type == 'content-group' && !params['search']" [content]="content">
        </app-category-details>
        <ng-container *ngIf="store">
          <app-giveaway-details *ngIf="content.type == 'giveaway'" [content]="content" [store]="store">
          </app-giveaway-details>
          <app-promotion-details *ngIf="content.type == 'promotion'" [content]="content" [store]="store">
          </app-promotion-details>
        </ng-container>
      </ng-container>

      <ng-container *ngIf="store">
        <app-issues
          *ngIf="storeName === 'issue' || storeName === 'ownedCatalogIssue' || storeName === 'favoritesCatalogIssue' || storeName === 'soldCatalogIssue'"
          [store]=" store" [storeName]="storeName"></app-issues>
        <app-series
          *ngIf="storeName === 'series' || storeName === 'ownedCatalogSeries' || storeName === 'favoritesCatalogSeries' || storeName ==='soldCatalogSeries'"
          [content]="content" [store]="store"></app-series>
        <app-user-owned-detail *ngIf="storeName === 'userOwnedDetail' || storeName === 'userSoldDetail'"
          [store]="store"></app-user-owned-detail>
        <app-user-starred *ngIf="storeName === 'userStarred'" [store]="store"></app-user-starred>
        <app-content-group *ngIf="storeName === 'contentGroupItem' || storeName === 'dashboardSectionItem'"
          [content]="content" [store]="store">
        </app-content-group>
      </ng-container>
    </ion-content>
  </ng-container>
  <ng-container *ngIf="subscriptionRequired">
    <app-subscribe-prompt></app-subscribe-prompt>
  </ng-container>
</ion-content>

But if you had some suggestions to what would make the rendering of this page more efficient that would be awesome.

liamdebeasi commented 3 years ago

One idea is to use CSS grid instead of ion-grid, ion-row, and ion-col since those all have JavaScript involved. Another option would be to minimize the conditional rendering as best you can. Angular is likely running those ngIf statements multiple times which can impact performance. Finally, you could try using the OnPush change detection strategy.

selected-pixel-jameson commented 3 years ago

Thank you I will look into these.

AE1NS commented 3 years ago

We had a similar issue. Make sure that your app (or entry) component + the page component itself does not have changeDetectionStrategy OnPush.

https://ionicframework.com/docs/angular/lifecycle#angular-life-cycle-events

Components that use ion-nav or ion-router-outlet should not use the OnPush change detection strategy. Doing so will prevent lifecycle hooks such as ngOnInit from firing. Additionally, asynchronous state changes may not render properly.

liamdebeasi commented 3 years ago

Thank you I will look into these.

Sounds good, let me know how it goes.

selected-pixel-jameson commented 3 years ago

I have determined that this is directly related to the view being rendered during the navigation animation. If I set this up so that the content is explicitly rendered in ionViewDidEnter() there is no lag. If I set it up to render in ionViewWillEnter() it happens. The interesting part is even if I take out a huge amount of the conditionals and checks in my templates it still will lag.

liamdebeasi commented 3 years ago

ionViewWillEnter will fire a few frames before the transition starts, so the initial render should fit into that window there otherwise you risk the render bleeding over into the animation. This can be tricky to control since Angular will sometimes render your page multiple times.

One other idea is to use ion-skeleton-text in your component and then move your render logic to ionViewDidEnter that way there is something in your view and your content does not pop in. Without seeing your code it is hard to say what else you could do to optimize.

I think the performance issue here seems to be more related to the app implementation rather than Ionic Framework, though there could be an iOS regression which is also impacting the performance.

I think it might make sense to move this discussion to the Ionic Forums by creating a new post there for additional debugging assistance, but let me know if you have any additional questions before I close this issue.

selected-pixel-jameson commented 3 years ago

I just want to state once again that this was working fine prior to iOS 14.6. So while there could be performance optimizations done here it seems as if there had to have been a performance regression in iOS 14.6.

liamdebeasi commented 3 years ago

I understand that, but where I do not have a way of reproducing the issue myself it is really hard for me to make an informed decision as to whether or not this is an iOS bug.

You could also try running a Timeline/Performance test in Safari Dev Tools to see which tasks are taking up the most time (I.e. is it a long running JS task? A lot of additional paints? Something else?)

I am going to close this issue for now as there is nothing actionable on the Ionic side, but if you are able to provide a reproduction in a repo I am more than happy to take another look.

selected-pixel-jameson commented 3 years ago

FYI it seems as if the biggest issue happens when attempting to make an HTTP request during the navigation transition which determines the content that will be displayed on the page. I've seen this on pages with relatively simple templates.

For Example

<ion-header *ngIf="!hideHeader">
  <ion-toolbar>
    <ion-buttons slot="start">
      <ion-back-button></ion-back-button>
    </ion-buttons>
    <ion-title>Future Keys</ion-title>
  </ion-toolbar>
</ion-header>
<ion-content>
  <app-category-details *ngIf="category" [content]="category"></app-category-details>
  <ion-list *ngIf="category && category.dates.length">
    <ion-item button *ngFor="let item of category.dates; trackBy: trackByMethod" (click)="open(item)">
      <ion-grid>
        <ion-row class="ion-align-items-center">
          <ion-col size="10">
            <div class="item-title">
              {{ item.title }}
              <span class="hasUpdate" *ngIf="item.hasUpdate">Update</span>
            </div>
          </ion-col>
          <ion-col size="2" *ngIf="item.issueCount">
            <span class="count-circle">{{ item.issueCount }}</span>
          </ion-col>
        </ion-row>
      </ion-grid>
    </ion-item>
  </ion-list>
  <ng-container *ngIf="subscriptionRequired">
    <app-subscribe-prompt></app-subscribe-prompt>
  </ng-container>
</ion-content>
import { Component, OnInit, Input } from "@angular/core";
import { Category } from "src/app/core/models/responses/category";
import { ActivatedRoute, Router } from "@angular/router";
import * as moment from "moment";
import { ContentService } from "src/app/core/http/content/content.service";
import { Subscription } from "rxjs";
import { MessageService } from "src/app/core/services/common/message/message.service";
import { take } from "rxjs/operators";

@Component({
  selector: "app-content-dates",
  templateUrl: "./content-dates.page.html",
  styleUrls: ["./content-dates.page.scss"],
})
export class ContentDatesPage implements OnInit {
  category: Category;
  hideHeader: boolean;
  subscriptionRequired: boolean;
  messageSubscription: Subscription;
  id: number;

  constructor(
    public activatedRoute: ActivatedRoute,
    public router: Router,
    public contentService: ContentService,
    public messageService: MessageService
  ) {

  }

  ngOnInit() {
    this.activatedRoute.params.subscribe((params) => {
      this.id = params["id"];
      this.hideHeader = this.activatedRoute.snapshot.data["hideHeader"];
    });

    this.messageSubscription = this.messageService
      .getMessage()
      .subscribe((message) => {
        if (message && message.text === "reload:content") {
          this.getContent();
        }
      });
  }

  ionViewWillEnter() {
    let content = history.state.content;
    if (!content) {
      this.getContent();
    } else {
      this.category = content
    }

  }

  ngOnDestroy() {
    this.messageSubscription.unsubscribe();
  }

  getContent() {
    this.contentService.getByTypeId("categories", this.id).pipe(take(1)).subscribe(
      (response) => {
        this.subscriptionRequired = false;
        this.category = new Category(response);
      },
      (error) => this.handleContentError(error)
    );
  }

  handleContentError(error) {
    this.category = null;
    this.subscriptionRequired = error.code === "1005" ? true : false;
  }

  open(item: any) {
    let startDate = moment(item.startDate).format("MM/DD/YYYY");
    let endDate = moment(item.endDate).format("MM/DD/YYYY");
    let dateRange = `${startDate}-${endDate}`;

    let segments = this.activatedRoute.snapshot["_urlSegment"].segments || null;
    let path = "/";
    if (segments) {
      path = `/${segments[0]}/${segments[1]}/category`;
    }

    this.router.navigate([path, this.category.id, "dateRanges", dateRange], {
      state: { subtitle: item.title, content: item },
      queryParams: { viewHistoryKey: item.date },
    });
  }

  trackByMethod(index: number, el: any): number {
    return el.date;
  }
}

Note let content = history.state.content. I had to resort to passing the content for pages in via router.navigate

this.router.navigate([path, this.category.id, "dateRanges", dateRange], {
      state: { subtitle: item.title, content: item },
      queryParams: { viewHistoryKey: item.date },
    });

I got lucky in the fact that most of the content that was in the lists or top level items was being passed down. The interesting part is that I am making other HTTP requests that aren't directly executed in the component but other Store Services, which don't seem to affect this at all. It's only when referencing this particular service, which is built out just like the other services.

 this.contentService.getByTypeId("categories", this.id).pipe(take(1)).subscribe(
      (response) => {
        this.subscriptionRequired = false;
        this.category = new Category(response);
      },
      (error) => this.handleContentError(error)
    );

This particular service is being cached with redis and the response times are typically under 200ms if not considerably lower like 50ms.

ionitron-bot[bot] commented 3 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.

liamdebeasi commented 3 years ago

Another developer was able to provide a code reproduction of this issue in https://github.com/ionic-team/ionic-framework/issues/23732 that I used to reproduce the issue on my end. The issue you are experiencing is a WebKit bug, and I will be reporting it to Apple this week.

You can follow https://github.com/ionic-team/ionic-framework/issues/23732 for updates.