bug: disconnectedCallback is not always called when it should be

[joewoodhouse]

Prerequisites

• I have read the Contributing Guidelines.
• I agree to follow the Code of Conduct.
• I have searched for existing issues that already report this problem, without success.

Stencil Version

3.0.1

Current Behavior

There are some very simple circumstances where a component's disconnectedCallback will not be called when it should be (i.e. when the element is moved or removed). This can lead to resource leaks if the user is attaching event listeners or doing other operations in their connectedCallback that they then hope to tidy up in the disconnectedCallback (e.g. adding event listeners).

Expected Behavior

The disconnectedCallback callback should be called consistently, inline with how a "native" CustomElement behaves.

System Info

No response

Steps to Reproduce

Simple adding then removing of element

const elm = document.createElement('my-stencil-component');

document.body.appendChild(elm);
// connectedCallback is now called

elm.remove();
// disconnectedCallback will NOT be called

Moving an element

const elm = document.createElement('my-stencil-component');

document.body.appendChild(elm);
// connectedCallback is now called

// Create another element to move ours into
const container = document.createElement('div');
document.body.appendChild(container);

// Move our element
container.appendChild(elm);
// First connectedCallback and disconnectedCallback *should* be called, but are not.

container.remove()
// disconnectedCallback is NOT called

The reproduction repository performs the above steps, but also does so alongside a "native" non-Stencil custom element, to show the "correct" behaviour. When you run the repository, every 1 second the above operations are run against both a "native" and a stencil component, and the counters for connected and disconnected callbacks are displayed. You will see that the native element performs correctly, calling connectedCallback and disconnectedCallback 2 times each. However the Stencil element only calls connectedCallback once per cycle, and NEVER calls disconnectedCallback.

Code Reproduction URL

https://github.com/joewoodhouse/stencil-disconnectedcallback-issues

Additional Information

Again I found this issue whilst debugging huge resource leaks in our Ionic/Capacitor application which is causing our app to crash regularly for our users after moderate usage.

The issue I believe is in the Stencil's patchdisconnectedCallback. The platform of course does call this method, it's just the patched method in the Stencil runtime does then not call the components own implementation.

https://github.com/ionic-team/stencil/blob/main/src/runtime/disconnected-callback.ts#L11

Here the code detects that we're lazy loading and takes the instance to be hostRef.$lazyIntance$. However in the cases above, $lazyInstance$ will not yet be set as lazy loading hasn't completed. The code in this case should wait for hostRef.$onReadyPromise$ to resolve before attempting to call the disconnectedCallback. I will provide a PR with this version shortly.

Note in all the above I have focused on disconnectedCallback but it should also all apply for componentDidUnload which is called (or in this case not) at the same time.

[rwaskiewicz]

Thanks @joewoodhouse!

I've confirmed this isn't working as expected based on the reproduction case and am going to get this labeled so that the team and I can discuss

[dobrek]

any updates on this issue? is this proposed fix taken into consideration to be merged soon?

[tanner-reits]

@joewoodhouse 's PR to resolve this issue has been merged and will be a part of the next Stencil release!

[tanner-reits]

This was resolved in Stencil v4.0.3. Please re-open this issue if you are still experiencing issues after updating.