Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| } | ||
|
|
||
| disconnectedCallback() { | ||
| if (this.keyboardCtrlPromise) { |
There was a problem hiding this comment.
Maybe this doesn't need to be done on this PR, but I feel like we do this same code in so many places that we should consider making it a utility.
There was a problem hiding this comment.
It looks like this particular pattern is only specifically done in tab bar and footer (unless I'm missing something?), so I'm not sure if that's worth extracting right now
There was a problem hiding this comment.
I mostly meant the code that is destroying like this:
ionic-framework/core/src/components/select/select.tsx
Lines 376 to 379 in 1c89cf0
We also disconnect in a lot of places:
ionic-framework/core/src/components/select/select.tsx
Lines 382 to 385 in 1c89cf0
It seems like we could make this into a utility or something, but that's not an issue for your PR.
There was a problem hiding this comment.
Let's make a ticket to circle back on adding the utility.
Issue number: resolves internal
What is the current behavior?
When
ion-tab-baris rapidly mounted and unmounted, a race condition in connectedCallback can cause the keyboard controller to be created after the component has been disconnected. This results in orphaned event listeners (keyboardWillShow,keyboardWillHide) on the window object that are never cleaned up, causing a memory leak.What is the new behavior?
The keyboard controller is now properly destroyed in all scenarios:
The promise tracking pattern ensures only the most recent async operation assigns its result
Does this introduce a breaking change?
Other information
Current dev build:
I was unable to find a way to create tests that accurately identified if this problem was occurring. Memory leaks are notoriously difficult to created automated tests for. I ultimately removed my previous attempts because I didn't want to give a false sense of security.