Stencil: Premature componentDidUnload for nested custom elements

Created on 24 Apr 2018 · 6Comments · Source: ionic-team/stencil

Stencil version:

 @stencil/[email protected]

I'm submitting a:

[ x ] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or https://stencil-worldwide.slack.com

Current behavior:

This seems specific to nesting a shadow: true component in the template of another shadow: true component. When the parent component is moved in the DOM (via append), the parent element does _not_ run componentDidUnload() but the embedded element does. If both components are set to shadow: false then componentDidUnload is not run on either.

Expected behavior:

Should have the same behavior for componentDidUnload in the parent and child for shadow or non-shadow elements.

Steps to reproduce:

Create shadow component B that uses shadow component A as part of its template
Add shadow component B to the body
Re-append B somewhere else on the body

Related code:

// insert any relevant code here
import { Component } from '@stencil/core';

@Component({
  tag: 'c-a',
  shadow: true
})
export class ComponentA {
  componentDidLoad() { console.log('A loaded');  }
  componentDidUnload() { console.log('A unloaded'); }
  render() {
    return <slot />;
  }
}

@Component({
  tag: 'c-b',
  shadow: true
})
export class ComponentB {
  componentDidLoad() { console.log('B loaded');  }
  componentDidUnload() { console.log('B unloaded');  }
  render() {
    return (
      <c-a>
        Component B
        <input type="checkbox" />
      </c-a>
    );
  }
}

<!DOCTYPE html>
<html dir="ltr" lang="en">
<head>
  <script>
    function moveIt() {
      var component = document.body.querySelector('c-b');
      document.body.append(component);
    }
  </script>
</head>
<body>
  <c-b></c-b>
  <button type="button" onclick="moveIt()">Move it!</button>
</body>
</html>

Other information:

So sadly, the inner component will run its cleanup code when the parent is moved. If both elements are switched to shadow: false then everything plays out fine. Is component A supposed to unload?

Edit: Thought I should mention non-shadow-dom browsers are not affected.

bug stale issue

Source

cary-smith

All 6 comments

Thanks for opening the issue. You're right, the lifecycle methods should always fire in the same order whether the components are using shadow or not. We'll get it fixed up, thanks

adamdbradley on 26 Apr 2018

👍1

Out of curiosity, what would be the 'proper' cycle? I am a bit fuzzy on connected (first only?) vs adopted (whenever moved via append?) callbacks and how that may affect these lifecycles.

cary-smith on 26 Apr 2018

We don't used connectedCallback since the same instance of a node could get moved around. Instead we have componentWillLoad and componentDidLoad, which takes into account a node could get removed from the dom and added back in. Additionally, componentWillLoad and componentDidLoad take into account if the child components have loaded or not. I'll admit our docs are lacking at the moment, so we'll put this on our list to improve. thanks

adamdbradley on 26 Apr 2018

👍1

@cary-smith I added a karma test that tests both shadow dom and the slot polyfill: https://github.com/ionic-team/stencil/commit/93f46dcb651f4d814cae940295daaca8cd7393b0 Looks like the test is passing, so maybe it was fixed recently or I didn't create the test correct to your sceanrio. Would you be able to take a look and make sure it's covering your scenario? Thanks

adamdbradley on 26 Apr 2018

So the test you have works well for verifying when a component is added/removed in its entirety. Unfortunately the scenario I am looking to address is exactly what you mentioned: a node being moved around like in a drag-n-drop.

The rough code I have in the html block above is the simplest version I could generate that exhibited the same behavior. It does not remove the component, but simply re-appends it to the body. What I am seeing in 0.7.26 & 0.7.27-2 is the parent ('c-b') component's componentDidUnload does not fire when it is re-appended, but it's embedded template ('c-a') component's does.

So maybe...

For test/karma/src/lifecycle-unload/index.html

<script src="/build/app.js"></script>
<div id="lifecycle-unload-results"></div>
<hr>
<lifecycle-unload-a></lifecycle-unload-a>
<lifecycle-unload-root></lifecycle-unload-root>

For test/karma/src/lifecycle-unload/cmp-root.tsx

import { Component, State } from '../../../../dist/index';

@Component({
  tag: 'lifecycle-unload-root'
})
export class LifecycleUnloadRoot {
  @Element() elm: HTMLElement;

  testClick() {
    const {body} = this.elm.ownerDocument;
    body.append(body.querySelector('lifecycle-unload-a'));
  }

  render() {
    return (
      <div>
        <button onClick={this.testClick.bind(this)}>Move</button>
      </div>
    );
  }
}

Not sure on the accuracy of that as I have not run it, but the movement of (not necessarily the removal of) the element and when unloads are called seems problematic.

cary-smith on 26 Apr 2018

Thanks for the issue! This issue is being closed due to inactivity. If this is still an issue with the latest version of Stencil, please create a new issue and ensure the template is fully filled out.

Thank you for using Stencil!