Svelte 5: `{#if}` blocks are not tree shaken... shook?

[brunnerh]

Describe the bug

It can be useful to have parts of a component only render during development, this can be achieved via build-time constants or e.g. SvelteKit's static env variable replacement.

Unfortunately, the structure of Svelte 5's output prevents tools like Rollup from making these optimizations.
This leads to unnecessary code being included in the production build and potentially private internals being leaked.

Reproduction

<script>
	import DebugTools from './DebugTools.svelte';
</script>

Visible text

{#if false}
	DEBUG MODE
	<DebugTools />
{/if}

<!-- DebugTools.svelte -->
[Debug Tools]

Svelte 4

Probably relevant optimizable output:

let if_block = false && create_if_block(ctx);

Rollup output contains neither inline text content nor the import of the debug tools component.

Code

import { SvelteComponent, init, safe_not_equal, text, empty, insert, noop, transition_in, transition_out, detach } from 'svelte/internal';
import 'svelte/internal/disclose-version';

/* App.svelte generated by Svelte v4.2.18 */

function create_fragment(ctx) {
	let t;
	let if_block_anchor;
	let current;
	let if_block = false;

	return {
		c() {
			t = text("Visible text\n\n");
			if_block_anchor = empty();
		},
		m(target, anchor) {
			insert(target, t, anchor);
			insert(target, if_block_anchor, anchor);
			current = true;
		},
		p: noop,
		i(local) {
			if (current) return;
			transition_in(if_block);
			current = true;
		},
		o(local) {
			transition_out(if_block);
			current = false;
		},
		d(detaching) {
			if (detaching) {
				detach(t);
				detach(if_block_anchor);
			}
		}
	};
}

class App extends SvelteComponent {
	constructor(options) {
		super();
		init(this, options, null, create_fragment, safe_not_equal, {});
	}
}

export { App as default };

Rollup REPL

Svelte 5

Probably relevant unoptimizable output:

$.if(node, () => false, ($$anchor) => { ... }

REPL

Rollup output contains both text content and the import (inlined).

Code

import * as $ from 'svelte/internal/client';

var root$1 = $.template(`[Debug Tools]`, 1);

function DebugTools($$anchor) {
	$.next();

	var fragment = root$1();

	$.append($$anchor, fragment);
}

var root_1 = $.template(`DEBUG MODE <!>`, 1);
var root = $.template(`Visible text <!>`, 1);

function App($$anchor) {
	$.next();

	var fragment = root();
	var node = $.sibling($.first_child(fragment, true));

	$.if(node, () => false, ($$anchor) => {
		var fragment_1 = root_1();
		var node_1 = $.sibling($.first_child(fragment_1, true));

		DebugTools(node_1);
		$.append($$anchor, fragment_1);
	});

	$.append($$anchor, fragment);
}

export { App as default };

Rollup REPL

Logs

No response

System Info

REPL / 5.0.0-next.220

Severity

annoyance

[paoloricciuti]

Ouch this hurts...i mean for very simple cases is probably optimisable anyway. If we can detect that is not dynamic we could just do

false && $.if(node, ()=>false)

and it seems to work and it seems to work even in more complex situations

I just wonder if this could lead to false positives.

[paoloricciuti]

Oh also this is not actually true if there's an else branch too.

[paoloricciuti]

Another option is to wrap every branch in the right test once again...it will be a bit more runtime overhead but to be fair i would prefer an extra if check to a non treeshaken if

[paoloricciuti]

Another option is to wrap every branch in the right test once again...it will be a bit more runtime overhead but to be fair i would prefer an extra if check to a non treeshaken if

However this will mess if there are function calls in the condition because they would be called twice. Maybe we can skip this if there are function calls (since it will never be tree-shaken away anyway?). But that could mess with something like dev && will_never_be_called()

[trueadm]

I'm not really sure how valuable this is outside of maybe optimising for specific known constants like DEV and BROWSER. If we output the condition before the block, we also need to output the else conditions and then we end up adding quite a lot of runtime code for something that likely rarely provides any real benefit.

[paoloricciuti]

I'm not really sure how valuable this is outside of maybe optimising for specific known constants like DEV and BROWSER. If we output the condition before the block, we also need to output the else conditions and then we end up adding quite a lot of runtime code for something that likely rarely provides any real benefit.

The problem i see is that it's kinda expected so one might do it being convinced that that branch will be excluded from the prod bundle only to find out that it is not. And i don't think it's rarely useful, dealing with something based on some env variable i think it's quite common after an application grows a bit.

[trueadm]

I'm not really sure how valuable this is outside of maybe optimising for specific known constants like DEV and BROWSER. If we output the condition before the block, we also need to output the else conditions and then we end up adding quite a lot of runtime code for something that likely rarely provides any real benefit.

The problem i see is that it's kinda expected so one might do it being convinced that that branch will be excluded from the prod bundle only to find out that it is not. And i don't think it's rarely useful, dealing with something based on some env variable i think it's quite common after an application grows a bit.

I don't doubt it's useful, but at the same time it's odd that if you had {#if BROWSER} and then you changed it to {#if is_in_view() && BROWSER} that it would no longer insert the condition beforehand. Furthermore, what about the else case?

[paoloricciuti]

I don't doubt it's useful, but at the same time it's odd that if you had {#if BROWSER} and then you changed it to {#if is_in_view() && BROWSER} that it would no longer insert the condition beforehand. Furthermore, what about the else case?

Oh yeah absolutely, that's the problem I was speaking of before, I didn't actually find a solution yet. For the else branch we can just wrap everything in !test...I have a basic implementation locally and it works but there's the problem of functions call that I don't know how to overcome

[paoloricciuti]

Unless we remove the function call and just compile to if-else?

[trueadm]

Unless we remove the function call and just compile to if-else?

You need to show me an example, I'm struggling to see how that would work.

[paoloricciuti]

Unless we remove the function call and just compile to if-else?

You need to show me an example, I'm struggling to see how that would work.

Uh scratch that I thought the if function code was a bit simpler.

[Azarattum]

Couldn't this be handled the Svelte compiler itself (without relying on Rollup)? If the condition is constant (doesn't have any $state), it can be computed beforehand, right?

[paoloricciuti]

Couldn't this be handled the Svelte compiler itself (without relying on Rollup)? If the condition is constant (doesn't have any $state), it can be computed beforehand, right?

Probably, but it's a huge and complex code that needs to be written and maintained so if we could rely on battle tested roll-up for this it would be better. We will probably start implementing this looking and very simple expressions and then maybe implement more sophisticated in house tree shaking

[NickAcPT]

I'd love to see this tackled before a full Svelte 5 release.
I'm obviously an outsider who isn't aware of all the intricacies of the Svelte compiler, but as a Svelte user, I would at least consider this to be some sort of regression that breaks (my) expectations of the compiler output.

It does make sense to rely on Rollup instead of rolling your own _{(no pun intended)}.
I wonder if we could get Rollup to "inline" the block_if calls to somehow look through them and hopefully optimize them better?

[trueadm]

I'd love to see this tackled before a full Svelte 5 release. I'm obviously an outsider who isn't aware of all the intricacies of the Svelte compiler, but as a Svelte user, I would at least consider this to be some sort of regression that breaks (my) expectations of the compiler output.

It does make sense to rely on Rollup instead of rolling your own (no pun intended). I wonder if we could get Rollup to "inline" the block_if calls to somehow look through them and hopefully optimize them better?

It’s not going to happen before we release 5.0. It’s something we’ll look into later.