Fix onunload

[Eggsecuter]

Problem

Onunload does not get called when a component gets reloaded or updated.
This is crucial as connections or event subscribers can't be closed properly.

Quick fixed it.

Don't know if this is properly implemented but after testing it a bit, I'm confident it works in any case.

Component.prototype.reload = async function () {
	await this.unload(); <-------------- !!!
	await this.onload();

		if (this.child) {
			await this.child.reload();
		}

	await this.update();
}

Component.prototype['addToElement'] = function addToElement(item, element: Node) {
	if (item instanceof Node) {
		element.appendChild(item);
	} else if (Array.isArray(item)) {
		for (let child of item) {
			this.addToElement(child, element);
		}
	} else if (item instanceof Component) {
		const placeholder = item.renderLoader();

		element.appendChild(placeholder);

		item.parent = this;

		(async () => {
			await item.onunload(); <-------------- !!!
			await item.onload();

			const child = item.render();
			item.rootNode = child;

			element.replaceChild(child, placeholder);
		})();
	} else if (item !== false && item !== undefined && item !== null) {
		element.appendChild(document.createTextNode(item));
	}
}

Component.prototype.host = async function host(parent: Node) {
	await this.onunload(); <-------------- !!!
	await this.onload();

	const root = this.render();
	this.rootNode = root;
	this.parent = null;

	parent.appendChild(root);
}

[levvij]

Unload should not be called when the component is updated, as this only updates the rendered view, without loading new data.

Reloading should call unload tho, please make a PR.

[levvij]

Host & AddToElement do not need an unload before the load call.

[Eggsecuter]

Then why is onload called but onunload isn't. Doesn't make sense to me.

I had to add onunload to AddToElement so if a componenet gets reloaded every "inline" component that isn't a direct child also gets properly unloaded.

Example:

render() {
    return <ui-page>{new ExampleComponent()}</ui-page>
}

ExampleComponenet will not get reloaded when PageComponenet is.

[levvij]

Let's use this example:

class ParentComponent extends Component {
    render() {
        return <ui-parent>{new ChildComponent()}</ui-parent>;
    }

    load() {
        console.log(`load parent`);
    }

    unload() {
        console.log(`unload parent`);
    }

    ...
}

class ChildComponent extends Component {
    static instanceCounter = 1;

    instance: number;

    constructor() {
        super();

        this.instance = ChildComponent.instanceCounter++;
    }

    load() {
        console.log(`load child ${this.instance}`);
    }

    unload() {
        console.log(`unload child ${this.instance}`);
    }

    ...
}

const parent = new ParentComponent();
parent.host(document.body); // add it to the body
// output: load parent
// output: load child 1

parent.update();
// output: load child 2

parent.reload();
// output: unload parent
// output: load parent
// output: load child 3

The ChildComponent currently does not receive a unload call and it will not be reloaded, but a new instance is created and its load is called.

Your quick-fix does not work, as it just calls unload on the new child component. We'd need to keep track of all child components and call unload on all of them when they are removed from the DOM.

vldom focuses on a minimal implementation which does not do any weird tracking of elements, thus the unload should be omitted. Unexpected behaviour will emerge as soon as we start to any tracking, because how do we handle the following cases:

• An html element is removed in code using removeChild, but there is a component inside that element - do we unload it? Or should it be unloaded when the parent is unloaded?
• A component A is passed as an argument to a component B, and A is rendered inside B, do we unload it when B is unloaded?

We'd need to add more and more shims and checks to keep all the cases handled, thus I chose to not add any further tracking to unload child elements.

[levvij]

After reconsideration with @Eggsecuter , there is a way to unload components without tracking them: Call unload on all components in before calling replace on update.

load is called on all components currently, load should only be called if it was not already present