Skip to content

Unified create/claim component construction #4219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mdempsky opened this issue Jan 6, 2020 · 2 comments
Closed

Unified create/claim component construction #4219

mdempsky opened this issue Jan 6, 2020 · 2 comments

Comments

@mdempsky
Copy link
Contributor

mdempsky commented Jan 6, 2020

Currently compiling a component emits a lot of boring DOM mutation logic. For example, even the simple svelte.dev/repl hello world example (when hydration is enabled) emits:

		c() {
			h1 = element("h1");
			t0 = text("Hello ");
			t1 = text(name);
			t2 = text("!");
		},
		l(nodes) {
			h1 = claim_element(nodes, "H1", {});
			var h1_nodes = children(h1);
			t0 = claim_text(h1_nodes, "Hello ");
			t1 = claim_text(h1_nodes, name);
			t2 = claim_text(h1_nodes, "!");
			h1_nodes.forEach(detach);
		},
		m(target, anchor) {
			insert(target, h1, anchor);
			append(h1, t0);
			append(h1, t1);
			append(h1, t2);
		},

I think this could instead be compiled as something like:

	c(ctx) {
		h1 = element(ctx, "h1", {}, (h1_ctx) => {
			text(h1_ctx, "Hello ");
			text(h1_ctx, name);
			text(h1_ctx, "!");
		});
	},

The context variable ctx would allow abstracting away the normal create/mount mode (i.e., appending to target before anchor) or in rehydrate's claim/mount mode (i.e., appending to target, but trying to reuse its existing children).

The convention would be nodes would be immediately inserted into the DOM in the order they're constructed, so there's no separate append calls necessary. Also, in rehydrate mode, it would prefer claiming existing nodes if possible and any leftover unclaimed nodes would be automatically detached, so there's no need for the h1_nodes.forEach(detach); call. Finally, because the text nodes are automatically appended to the DOM, there's no need to save them as t0/t1/t2 anymore. (h1 still needs to be saved to implement detach.)

For example, when compiling without rehydration support, element and text might be implemented something like:

function construct(fn, target, anchor) {
    const ctx = [target, archor];
    fn(ctx);
}
function emit(ctx, node) {
    const [target, anchor] = ctx;
    target.appendChild(node, anchor);
}

function element(ctx, name, attributes, children) {
    const node = document.createElement(name);
    set_attributes(node, attributes);
    construct(children, node);  // Recursively construct children with new context
    emit(ctx, node);
}
function text(ctx, data) {
    emit(ctx, document.createTextNode(data));
}

Alternatively, by simply tweaking construct and emit, we could defer DOM operations until all of a node's children have been constructed (i.e., more like the current separation of create/mount):

function construct(fn, target, anchor) {
    const ctx = [];
    fn(ctx);
    for (let child of ctx) {
        target.appendChild(child, anchor);
    }
}
function emit(ctx, node) {
    ctx.push(node);
}

Edit: Or using document fragments:

function construct(fn, target, archor) {
    const ctx = document.createDocumentFragment();
    fn(ctx);
    target.appendChild(ctx, anchor);
}
function emit(ctx, node) {
    ctx.appendChild(node);
}

I plan on experimenting with the feasibility of this approach and its impact on JS bundle size.

My main question is whether there's a need for the current separation between node construction and mounting. E.g., is it needed for animation or something? As far as I can tell, it doesn't seem needed.

Filing an issue to describe the idea in case anyone has feedback on it (e.g., further room for improvement, or issues I'm overlooking that might prevent this approach from working at all).

@stale
Copy link

stale bot commented Dec 24, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Dec 24, 2021
@Rich-Harris
Copy link
Member

Svelte 5 fixes this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants