Call onMount when connected & clean up when disconnected for custom element

[hontas]

Relates to #1152
Resolves #2227

Solves the issue with function returned from onMount not getting called for customElements.

Introduces three changes:

1. Run onMount in connectedCallback
2. Register return values from onMount as on_disconnect and run them in disconnectedCallback
3. Not resetting on_mount after initial run

Benefits:

1. Props are initialised when onMount happens (no more tick().then(/**/)). Maps closer to mental model of a custom element.
2. We can clean up (prevent memory leaks) without changing how a normal Svelte component behaves. Note that onDestroy is still not called.
3. If component is disconnected and then reinserted, callbacks would run again, generating fresh clean up callbacks.

Example:

<svelte:options tag="x-tag"/>
<script>
  import { onMount } from 'svelte';

  export let disabled = false;

  onMount(() => {
    console.log('will be called by connectedCallback');
    console.log(disabled); // "" (empty string, if set like this: <x-tag disabled/>)

    return () => {
      console.log('will be called by disconnectedCallback');
    }
  });
</script>

[Rich-Harris]

Thank you. What do you think about the discussion in that issue though — about the fact that we actually don't want onDestroy callbacks to run when an element is merely disconnected? What if instead, only the return values from onMount callbacks were run in those situations, and onDestroy callbacks were restricted to when $destroy is explicitly called?

[hontas]

@Rich-Harris splendid!

Updated the PR so that on_mount-callbacks are run from connectedCallback, saving its return values as on_disconnect. 👈 This is then run from disconnectedCallback.
That way, if the component is reinserted the disconnect-callbacks are re-generated and can run again when disconnected.
And we get the added benefit that props are initialized at the time of onMount which I think simplifies the mental model when working with custom elements 😀

[jesper-bylund]

@tanhauhau is this being merged? Looks like it's waiting on you, AND that you've apporved. I'm confused. 😅

[Truffula]

@hontas I've been working on another solution to #2227 (see #5139) and comparing the impact on the various hooks as well as any top-level code. If the props are initialised before onMount is called, but not before the initial render or beforeUpdate, you can end up rendering the template without the props being available - i.e. you end up rendering twice, and if any props are mandatory you need to add extra logic for the case where they're not initialised yet.

[yannkost]

Can I ask if this issu is still being worked on?
The custom solution here, adding the following seems to work just fine for me..

class SubElement extends SvelteElement { // extend the Svelte custom element here
  disconnectedCallback() {
    this.$destroy()
  }
}

Still, I don't like the fact that I'm changing something in the svelte internals. Still, for some cases it is needed to call onDestroy to clean up after removing a custom element.

[hontas]

Can I ask if this issu is still being worked on?

Certainly @yannkost ;) It’s still waiting for feedback from the maintainers.
Regarding the custom solution, please see @Rich-Harris comment above and the thoughts in #1152 on how they don’t want destroy to be called from disconnectedCallback.

[antony]

Are the issues discussed above solved by #5139 ?

We should close one of these PRs, and focus on getting the other merged. Perhaps somebody who is closer to the issue (somebody who knows about custom elements) could clarify?

[Truffula]

@antony I don't think #5139 solves all of the issues raised in this PR, it deals with the creation lifecycle but not disconnectedCallback. IIRC this PR (#4522) doesn't prevent exports from initialising with the wrong value, so ultimately we probably need a bit of both.

[arackaf]

Don't worry - I'm not here asking for "when it'll be merged" :-)

Just wanna make sure I understand what this PR does, as it pertains to web components, and the comments @Rich-Harris made.

So, apparently, it's not unheard of for a web component to be disconnected, and then re-connected / re-attached to the dom. And so what this PR does is fire only the onUnMount handler the user can optionally return from onMount, when the web component is disconnected, but otherwise not destroy anything. And then if the wc were to be re-attached to the dom, then the user's onMount handler would fire de novo, and the whole process would repeat.

Is that about right?

[hontas]

@arackaf yes that’s the idea 💡

[arackaf]

@antony - per our discussion, I've pulled down this PR, and tested it locally. It appears to be a solid fix for web component connected / disconnected lifecycles, as they pertain to Svelte. Before, onMount would be called the moment the web component was created, and the unmount callback was never, ever called.

Now, with this PR, the onMount handler is called when the wc is connected to the dom, with the unMount handler being called when it's disconnected. Rinse, repeat. I doubt this fixes all outstanding wc issues, but it seems to fix a big one, and well. I'm confident enough to wager @Rich-Harris's life on it ;)

@hontas - your branch appears to be far behind master (I of course merged prior to testing). Would you mind merging and pushing? Heads up - expect one conflict in a test file.

[arackaf]

@hontas I see you just updated your branch - are also you ok fixing the issue Rich mentioned? I'm happy to if this is a busy / bad time for you.

[hontas]

@arackaf I'm on it!
@Rich-Harris thanks for the feedback. Did you mean from the init function? If so where is it obvious to us that it's a CE that's being rendered?

[arackaf]

@hontas - you're amazing. I can't answer your question, but in the off chance there's no easy way to implement what @Rich-Harris suggested, would

const isCustomElement = component instanceof SvelteElement;

work? I think that would be a lot more performant that the original code. (untested of course - just an idea - apologies if there's something dumb wrong with that)

[hontas]

@Rich-Harris updated PR with your suggestions 😃 please have a look if that's what you had in mind. I'm super excited!
Thank you @arackaf! I think I found another even more performant option 👍

[yannkost]

Do I understand this right when I say that the component will execute the callback fonction set up in onMount when disconnected (only if it's a custom élément)? (This is what I seem to understand in the code)

[arackaf]

@yannkost it’ll run the teardown function, which is returned from onMount, when the custom element disconnects. Likewise it’ll run the onMount function itself when the web component connects.

[arackaf]

@hontas thanks a ton for making this fix. Huge lifesaver.

I'm not by any means asking you to fix this, but since you seem to have interest in using Svelte for web components, just an fyi that I recently found this issue: #5989

Seems like the fix would be incredibly similar to what you did here, or at the very least, something for you to be aware of, for your future use of Svelte :)

Thanks again!