search

ionic-team / stencil

A toolchain for building scalable, enterprise-ready component systems on top of TypeScript and Web Component standards. Stencil components can be distributed natively to React, Angular, Vue, and traditional web developers from a single, framework-agnostic codebase.
https://stenciljs.com
Other
12.5k stars 783 forks source link

bug: conditional rendering with ionic component causes memory leak #5172

Open Lucas-Quinn-1163273 opened 9 months ago

Lucas-Quinn-1163273 commented 9 months ago

Prerequisites

Stencil Version

@stencil/core at all versions, 2.x, 3.x, 4.x and the latest 4.8.2. Haven't tried 1.x.

Current Behavior

I have identified two types of memory leaks so far. The first type occurs in the development environment and is caused by components like ion-button.

The second type is more complex, but more importantly, it happens in prod build too - this has been moved to https://github.com/ionic-team/stencil/issues/5181

Let's focus on the memory leak in the development environment in this issue.

The First Kind of Leaking

When rendering a component conditionally, certain components, such as ion-button, trigger a memory leak. If the component exists in a Stencil component and is rendered conditionally, the entire Stencil component fails to be garbage collected. For instance, wrapping ion-button in a Stencil component and rendering it conditionally prevents the garbage collection of the Stencil component.

While this type of leaking is limited to the development environment, distinguishing between development-only and production-related leaks is challenging. This ambiguity significantly impacts the development experience, requiring extensive time spent on debugging memory leaks. To determine whether a component is leaking, one must build and run the app in production mode, adding a considerable overhead to the debugging process.

Environment details:

To reproduce the issue:

  1. Run npm install.
  2. Execute npm run start.
  3. Click the button to observe the memory leak issue.
Screenshot 2023-12-15 at 11 16 56 AM Screenshot 2023-12-15 at 11 18 22 AM

Replace ion-button with other components to observe the same issue:

Expand to see components - [ ] ion-action-sheet - [ ] ion-accordion - [ ] ion-accordion-group - [ ] ion-alert - [ ] ion-badge - [ ] ion-breadcrumb - [ ] ion-breadcrumbs - [x] ion-button - [ ] ion-ripple-effect - [ ] ion-card - [ ] ion-card-content - [ ] ion-card-header - [ ] ion-card-subtitle - [ ] ion-card-title - [ ] ion-checkbox - [x] ion-chip - [ ] ion-app - [ ] ion-content - [x] ion-datetime - [x] ion-datetime-button - [ ] ion-picker - [ ] ion-fab - [x] ion-fab-button - [ ] ion-fab-list - [ ] ion-grid - [ ] ion-col - [ ] ion-row - [x] ion-infinite-scroll - [ ] ion-infinite-scroll-content - [ ] ion-icon - [ ] ion-input - [ ] ion-textarea - [ ] ion-item - [ ] ion-item-divider - [ ] ion-item-group - [ ] ion-item-sliding - [ ] ion-item-options - [x] ion-item-option - [ ] ion-label - [ ] ion-note - [ ] ion-list - [ ] ion-list-header - [ ] ion-avatar - [ ] ion-icon - [ ] ion-img - [ ] ion-thumbnail - [ ] ion-menu - [ ] ion-menu-button - [ ] ion-menu-toggle - [ ] ion-split-pane - [ ] ion-modal - [ ] ion-backdrop - [ ] ion-nav - [ ] ion-nav-link - [ ] ion-router-outlet - [ ] ion-route - [ ] ion-route-redirect - [x] ion-searchbar - [ ] ion-segment - [x] ion-segment-button - [x] ion-select - [x] ? ion-select-option with ion-select - [ ] ion-tabs - [x] ? ion-tab with ion-nav inside - [x] ion-tab-bar - [x] ion-tab-button - [ ] ion-toast - [ ] ion-toggle - [ ] ion-toolbar - [ ] ion-header - [ ] ion-footer - [ ] ion-title - [x] ?ion-buttons (must have ion-button, so it has an issue) - [ ] ion-back-button - [ ] ion-text

The Second Kind of Leaking

Moved to https://github.com/ionic-team/stencil/issues/5181

Expected Behavior

Detached elements should be garbage collected.

System Info

npx @stencil/core info

System: node 21.4.0
    Platform: darwin (23.2.0)
   CPU Model: Apple M2 (8 cpus)
    Compiler: /Users/[username]/.npm/_npx/0eea9657e3e2e6b9/node_modules/@stencil/core/compiler/stencil.js
       Build: 1702322155
     Stencil: 4.8.2 🐳
  TypeScript: 5.2.2
      Rollup: 2.42.3
      Parse5: 7.1.2
      Sizzle: 2.42.3
      Terser: 5.26.0

Tried different browsers, different systems, different node versions, different typescript versions too.

Steps to Reproduce

Clone the repo

To reproduce the issue:

  1. Run npm install.
  2. Execute npm run start.
  3. Click the button to observe the memory leak issue.

https://github.com/lucas-quinn/ionic-memory-leak

Additional Information

No response

rwaskiewicz commented 9 months ago

Hey :wave:

Thanks for the issue! I’ve taken an initial look at the reproduction case you’ve provided, and it does appear there is a memory leak of some type occurring here.

Before we move forward with this, can you help us out with a few things?

Can you do me a favor and split this issue into two, one for each type of leaking described? The reason I ask is:

Splitting the issue can be done by creating a new issue, copying the existing details from the second reported leak into the new issue.

From there, someone on the team will take a closer look at each issue and may follow up/request some additional information.

Thanks!

Lucas-Quinn-1163273 commented 9 months ago

Hey 👋

Thanks for the issue! I’ve taken an initial look at the reproduction case you’ve provided, and it does appear there is a memory leak of some type occurring here.

Before we move forward with this, can you help us out with a few things?

Can you do me a favor and split this issue into two, one for each type of leaking described? The reason I ask is:

  • It helps us track the work associated with investigations, root cause analysis, and testing fix(es)
  • It helps us ensure that we’re not conflating details from two (potentially separate) issues of the same type

Splitting the issue can be done by creating a new issue, copying the existing details from the second reported leak into the new issue.

From there, someone on the team will take a closer look at each issue and may follow up/request some additional information.

Thanks!

Thank you so much!

I split the second kind of leak into another one, #5181 . Since the second one affects production, it should be considered more important. However, this one occurs in the development environment, making it challenging to identify issues in production. Therefore, both are very important. I hope to hear from the team soon!

rwaskiewicz commented 9 months ago

Thanks @lucas-quinn !

I took the liberty of moving the list of Ionic Components to a collapsible section in the issue summary to minimize some additional scrolling, as well as adding another reference to the second issue there.

Can you do me a favor and for this issue, add a few screenshots of what you're seeing in terms of memory leaks? While I think we're on the same page, I'd like to make sure we're working on the issue/type of memory leak reported here. Thanks!

Lucas-Quinn-1163273 commented 9 months ago

@rwaskiewicz Thanks! I have added two screenshots. I'm using Microsoft Edge - Detached Elements here because Chrome doesn't offer this devtool to easily debug detached elements. But Chrome does offer memory profiling.

rwaskiewicz commented 9 months ago

Thanks @lucas-quinn! I've validated this bug with the reproduction case + the screenshots. I've labeled it to get it ingested into our backlog to prioritize.

mmben commented 4 months ago

I'm not 100% sure, but I think we may be running into this leak as well. Mostly it is just some ZoneAwarePromise objects that are retained, but sometimes we have HTMLElements as well. The retainers always point to the constructor in Line 101 of bootstrap-lazy.ts: // StencilLazyHost constructor(self: HTMLElement) {

We're running: @ionic/angular@8.0.0 @ionic/core@8.0.0 @stencil/core@4.16.0 on Node 20.12.2