Skip to content
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

More aggressive innerHTML optimization detection #2374

Closed
aweary opened this issue Apr 8, 2019 · 5 comments
Closed

More aggressive innerHTML optimization detection #2374

aweary opened this issue Apr 8, 2019 · 5 comments
Labels
compiler Changes relating to the compiler feature request perf temp-stale

Comments

@aweary
Copy link

aweary commented Apr 8, 2019

Svelte will fallback to using innerHTML if a block of markup is entirely static. This results in much less generated code. I noticed that if a single item in that block is dynamic, regardless of its position, the output is deopt'd and it falls back to just using element and append for everything.

Here is an example of having a single dynamic element in a list. This can potentially result in a lot more generated code, creating local variables in create_fragment that don't really need to exist.

Instead of falling back to the imperative DOM APIs could svelte detect if using a combination of both would be beneficial? In this example you could still use innerHTML to define the n - 1 list elements and then use element / append for the one dynamic tag. This seems like it would be easiest when the dynamic element is the first or last child, but I imagine you could also do this with element.innerHTML += "..."

@Rich-Harris
Copy link
Member

That's a good idea. I don't think we can do element.innerHTML += x, since that would break existing references, but we can do this sort of thing:

ul = element('ul');
ul.innerHTML = `<li>one</li><li>two</li>`;
li = element('li');
li.textContent = name;
li.insertAdjacentHTML('afterEnd', `'<li>four</li><li>five</li>`);

In this case, where we already know that name never changes (since it's not a prop, and isn't the subject of any assignments) we could actually go further still:

ul = element('ul');
ul.innerHTML = `<li>one</li><li>two</li><li>${name}</li><li>four</li><li>five</li>`;

A minifier ought to be able to turn that into a static string.

It's also true that in some cases we could do this sort of thing, if we did need references:

ul = element('ul');
ul.innerHTML = `<li>one</li><li>two</li><li>${ctx.name}</li><li>four</li><li>five</li>`;
t = ul.children[2].childNodes[0];

@benmccann
Copy link
Member

This seems potentially quite similar to #3898

@pngwn pngwn added compiler Changes relating to the compiler temp-stale and removed integrations labels Jun 27, 2021
@DaniAcu
Copy link

DaniAcu commented Sep 7, 2021

@Rich-Harris I was dinging into this stuff because I noticed that static elements are just appended in dom with innetHTML. So I wondering what are the security caution that we are taking care in the case of using it and what are the benefit of doing these instead to creating the elements (performance i guess, but I prefer ask to have a better understanding).

@benmccann
Copy link
Member

Note that this does particularly poorly when hydratable: true: #7226

@Rich-Harris
Copy link
Member

closing since Svelte 5 uses a completely different mechanism that essentially gives us this optimization for free

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler Changes relating to the compiler feature request perf temp-stale
Projects
None yet
Development

No branches or pull requests

6 participants