fix: reduce if block nesting (#17662)

* fix: reduce if block nesting

This reduces if block nesting similar to how we did it in #15250 (which got lost during the `await` feature introduction): If the if expression doesn't contain an await expression or is not dependent on a blocker that is not already resolved, then we can avoid creating a separate `$.if()` statement. The one trade-off is that we'll do re-invocations  for all the conditions leading up to the condition that matches. Therefore non-simple if expressions are wrapper in `$.derived` to avoid excessive recomputations.

closes #17659 (~320 markers in prod mode possible now; less in dev because of our "wrap this component with devtime info" method)
helps with #15200

* tweak

* feedback
pull/17696/head
Simon H 3 days ago committed by GitHub
parent 35c845e35d
commit bd5480b9d0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: reduce if block nesting

@ -24,4 +24,23 @@ export function IfBlock(node, context) {
context.visit(node.consequent);
if (node.alternate) context.visit(node.alternate);
// Check if we can flatten branches
const alt = node.alternate;
if (alt && alt.nodes.length === 1 && alt.nodes[0].type === 'IfBlock' && alt.nodes[0].elseif) {
const elseif = alt.nodes[0];
// Don't flatten if this else-if has an await expression or new blockers
// TODO would be nice to check the await expression itself to see if it's awaiting the same thing as a previous if expression
if (
!elseif.metadata.expression.has_await &&
!elseif.metadata.expression.has_more_blockers_than(node.metadata.expression)
) {
// Roll the existing flattened branches (if any) into this one, then delete those of the else-if block
// to avoid processing them multiple times as we walk down the chain during code transformation.
node.metadata.flattened = [elseif, ...(elseif.metadata.flattened ?? [])];
elseif.metadata.flattened = undefined;
}
}
}

@ -47,7 +47,11 @@ export function Fragment(node, context) {
const is_single_element = trimmed.length === 1 && trimmed[0].type === 'RegularElement';
const is_single_child_not_needing_template =
trimmed.length === 1 &&
(trimmed[0].type === 'SvelteFragment' || trimmed[0].type === 'TitleElement');
(trimmed[0].type === 'SvelteFragment' ||
trimmed[0].type === 'TitleElement' ||
(trimmed[0].type === 'IfBlock' &&
trimmed[0].elseif &&
/** @type {AST.IfBlock} */ (parent).metadata.flattened?.includes(trimmed[0])));
const template_name = context.state.scope.root.unique('root'); // TODO infer name from parent
/** @type {Statement[]} */

@ -1,4 +1,4 @@
/** @import { BlockStatement, Expression } from 'estree' */
/** @import { BlockStatement, Expression, IfStatement, Statement } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types' */
import * as b from '#compiler/builders';
@ -10,40 +10,75 @@ import { build_expression, add_svelte_meta } from './shared/utils.js';
*/
export function IfBlock(node, context) {
context.state.template.push_comment();
/** @type {Statement[]} */
const statements = [];
const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent));
const consequent_id = b.id(context.state.scope.generate('consequent'));
const has_await = node.metadata.expression.has_await;
const has_blockers = node.metadata.expression.has_blockers();
const expression = build_expression(context, node.test, node.metadata.expression);
// Build the if/else-if/else chain
let index = 0;
/** @type {IfStatement | undefined} */
let first_if;
/** @type {IfStatement | undefined} */
let last_if;
/** @type {AST.IfBlock | undefined} */
let last_alt;
statements.push(b.var(consequent_id, b.arrow([b.id('$$anchor')], consequent)));
for (const branch of [node, ...(node.metadata.flattened ?? [])]) {
const consequent = /** @type {BlockStatement} */ (context.visit(branch.consequent));
const consequent_id = b.id(context.state.scope.generate('consequent'));
statements.push(b.var(consequent_id, b.arrow([b.id('$$anchor')], consequent)));
let alternate_id;
// Build the test expression for this branch
/** @type {Expression} */
let test;
if (node.alternate) {
const alternate = /** @type {BlockStatement} */ (context.visit(node.alternate));
alternate_id = b.id(context.state.scope.generate('alternate'));
statements.push(b.var(alternate_id, b.arrow([b.id('$$anchor')], alternate)));
if (branch.metadata.expression.has_await) {
// Top-level condition with await: already resolved by $.async wrapper
test = b.call('$.get', b.id('$$condition'));
} else {
const expression = build_expression(context, branch.test, branch.metadata.expression);
if (branch.metadata.expression.has_call) {
const derived_id = b.id(context.state.scope.generate('d'));
statements.push(b.var(derived_id, b.call('$.derived', b.arrow([], expression))));
test = b.call('$.get', derived_id);
} else {
test = expression;
}
}
const render_call = b.stmt(b.call('$$render', consequent_id, index > 0 && b.literal(index)));
const new_if = b.if(test, render_call);
if (last_if) {
last_if.alternate = new_if;
} else {
first_if = new_if;
}
last_alt = branch;
last_if = new_if;
index++;
}
const has_await = node.metadata.expression.has_await;
const has_blockers = node.metadata.expression.has_blockers();
// Handle final alternate (else branch, remaining async chain, or nothing)
if (last_if && last_alt?.alternate) {
const alternate = /** @type {BlockStatement} */ (context.visit(last_alt.alternate));
const alternate_id = b.id(context.state.scope.generate('alternate'));
statements.push(b.var(alternate_id, b.arrow([b.id('$$anchor')], alternate)));
const expression = build_expression(context, node.test, node.metadata.expression);
const test = has_await ? b.call('$.get', b.id('$$condition')) : expression;
last_if.alternate = b.stmt(b.call('$$render', alternate_id, b.literal(false)));
}
// Build $.if() arguments
/** @type {Expression[]} */
const args = [
context.state.node,
b.arrow(
[b.id('$$render')],
b.block([
b.if(
test,
b.stmt(b.call('$$render', consequent_id)),
alternate_id && b.stmt(b.call('$$render', alternate_id, b.literal(false)))
)
])
)
b.arrow([b.id('$$render')], first_if ? b.block([first_if]) : b.block([]))
];
if (node.elseif) {
@ -67,7 +102,9 @@ export function IfBlock(node, context) {
//
// ...even though they're logically equivalent. In the first case, the
// transition will only play when `y` changes, but in the second it
// should play when `x` or `y` change — both are considered 'local'
// should play when `x` or `y` change — both are considered 'local'.
// This could also be a non-flattened elseif (because it has an async expression).
// In both cases mark as elseif so the runtime uses EFFECT_TRANSPARENT for transitions.
args.push(b.true);
}

@ -1,4 +1,4 @@
/** @import { BlockStatement, Expression, Statement } from 'estree' */
/** @import { BlockStatement, Expression, IfStatement, Statement } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types.js' */
import * as b from '#compiler/builders';
@ -9,23 +9,37 @@ import { block_close, block_open, block_open_else, create_child_block } from './
* @param {ComponentContext} context
*/
export function IfBlock(node, context) {
const test = /** @type {Expression} */ (context.visit(node.test));
const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent));
consequent.body.unshift(b.stmt(b.call(b.id('$$renderer.push'), block_open)));
const alternate = node.alternate
? /** @type {BlockStatement} */ (context.visit(node.alternate))
: b.block([]);
/** @type {IfStatement} */
let if_statement = b.if(/** @type {Expression} */ (context.visit(node.test)), consequent);
consequent.body.unshift(b.stmt(b.call(b.id('$$renderer.push'), block_open)));
let index = 1;
let current_if = if_statement;
let alt = node.alternate;
// Walk the else-if chain, flattening branches
for (const elseif of node.metadata.flattened ?? []) {
const branch = /** @type {BlockStatement} */ (context.visit(elseif.consequent));
branch.body.unshift(b.stmt(b.call(b.id('$$renderer.push'), b.literal(`<!--[${index++}-->`))));
current_if = current_if.alternate = b.if(
/** @type {Expression} */ (context.visit(elseif.test)),
branch
);
alt = elseif.alternate;
}
alternate.body.unshift(b.stmt(b.call(b.id('$$renderer.push'), block_open_else)));
// Handle final else (or remaining async chain)
const final_alternate = alt ? /** @type {BlockStatement} */ (context.visit(alt)) : b.block([]);
/** @type {Statement} */
let statement = b.if(test, consequent, alternate);
final_alternate.body.unshift(b.stmt(b.call(b.id('$$renderer.push'), block_open_else)));
current_if.alternate = final_alternate;
context.state.template.push(
...create_child_block(
[statement],
[if_statement],
node.metadata.expression.blockers(),
node.metadata.expression.has_await
),

@ -118,6 +118,19 @@ export class ExpressionMetadata {
return this.#get_blockers().size > 0;
}
/**
* @param {ExpressionMetadata} other
*/
has_more_blockers_than(other) {
for (const blocker of this.#get_blockers()) {
if (!other.#get_blockers().has(blocker)) {
return true;
}
}
return false;
}
is_async() {
return this.has_await || this.#get_blockers().size > 0;
}

@ -483,6 +483,8 @@ export namespace AST {
alternate: Fragment | null;
/** @internal */
metadata: {
/** List of else-if blocks that can be flattened into this if block */
flattened?: IfBlock[];
expression: ExpressionMetadata;
};
}

@ -9,14 +9,12 @@ import {
set_hydrating
} from '../hydration.js';
import { block } from '../../reactivity/effects.js';
import { HYDRATION_START_ELSE } from '../../../../constants.js';
import { BranchManager } from './branches.js';
// TODO reinstate https://github.com/sveltejs/svelte/pull/15250
import { HYDRATION_START, HYDRATION_START_ELSE } from '../../../../constants.js';
/**
* @param {TemplateNode} node
* @param {(branch: (fn: (anchor: Node) => void, flag?: boolean) => void) => void} fn
* @param {(branch: (fn: (anchor: Node) => void, key?: number | false) => void) => void} fn
* @param {boolean} [elseif] True if this is an `{:else if ...}` block rather than an `{#if ...}`, as that affects which transitions are considered 'local'
* @returns {void}
*/
@ -29,14 +27,28 @@ export function if_block(node, fn, elseif = false) {
var flags = elseif ? EFFECT_TRANSPARENT : 0;
/**
* @param {boolean} condition,
* @param {number | false} key
* @param {null | ((anchor: Node) => void)} fn
*/
function update_branch(condition, fn) {
function update_branch(key, fn) {
if (hydrating) {
const is_else = read_hydration_instruction(node) === HYDRATION_START_ELSE;
const data = read_hydration_instruction(node);
/**
* @type {number | false}
* "[" = branch 0, "[1" = branch 1, "[2" = branch 2, ..., "[!" = else (false)
*/
var hydrated_key;
if (data === HYDRATION_START) {
hydrated_key = 0;
} else if (data === HYDRATION_START_ELSE) {
hydrated_key = false;
} else {
hydrated_key = parseInt(data.substring(1)); // "[1", "[2", etc.
}
if (condition === is_else) {
if (key !== hydrated_key) {
// Hydration mismatch: remove everything inside the anchor and start fresh.
// This could happen with `{#if browser}...{/if}`, for example
var anchor = skip_nodes();
@ -45,22 +57,22 @@ export function if_block(node, fn, elseif = false) {
branches.anchor = anchor;
set_hydrating(false);
branches.ensure(condition, fn);
branches.ensure(key, fn);
set_hydrating(true);
return;
}
}
branches.ensure(condition, fn);
branches.ensure(key, fn);
}
block(() => {
var has_branch = false;
fn((fn, flag = true) => {
fn((fn, key = 0) => {
has_branch = true;
update_branch(flag, fn);
update_branch(key, fn);
});
if (!has_branch) {

@ -95,7 +95,12 @@ export function skip_nodes(remove = true) {
if (data === HYDRATION_END) {
if (depth === 0) return node;
depth -= 1;
} else if (data === HYDRATION_START || data === HYDRATION_START_ELSE) {
} else if (
data === HYDRATION_START ||
data === HYDRATION_START_ELSE ||
// "[1", "[2", etc. for if blocks
(data[0] === '[' && !isNaN(Number(data.slice(1))))
) {
depth += 1;
}
}

@ -0,0 +1,12 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
mode: ['async-server', 'hydrate', 'client'],
ssrHtml: `bar blocking`,
async test({ assert, target }) {
await tick();
assert.htmlEqual(target.innerHTML, 'bar blocking');
}
});

@ -0,0 +1,21 @@
<script>
let foo = $state(false);
let blocking = $derived(await foo);
let bar = Promise.resolve(true);
</script>
{#if foo}
foo
{:else if await bar}
bar
{:else}
else
{/if}
{#if foo}
foo
{:else if !blocking}
blocking
{:else}
else
{/if}

@ -0,0 +1,7 @@
<script>
let { x } = $props();
console.log(x);
$effect(() => console.log('$effect: '+ x))
</script>
{x}

@ -0,0 +1,47 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
skip: true, // this fails on main, too; skip for now
async test({ assert, target, logs }) {
const [x, y, resolve] = target.querySelectorAll('button');
x.click();
await tick();
assert.deepEqual(logs, ['universe']);
y.click();
await tick();
assert.deepEqual(logs, ['universe', 'world', '$effect: world']);
assert.htmlEqual(
target.innerHTML,
`
<button>x</button>
<button>y++</button>
<button>resolve</button>
world
`
);
resolve.click();
await tick();
assert.deepEqual(logs, [
'universe',
'world',
'$effect: world',
'$effect: universe',
'$effect: universe'
]);
assert.htmlEqual(
target.innerHTML,
`
<button>x</button>
<button>y++</button>
<button>resolve</button>
universe
universe
universe
`
);
}
});

@ -0,0 +1,28 @@
<script>
import Child from './Child.svelte';
let x = $state('world');
let y = $state(0);
let deferred = [];
function delay(s) {
const d = Promise.withResolvers();
deferred.push(() => d.resolve(s))
return d.promise;
}
</script>
<button onclick={() => x = 'universe'}>x</button>
<button onclick={() => y++}>y++</button>
<button onclick={() => deferred.shift()()}>resolve</button>
{#if x === 'universe'}
{await delay(x)}
<Child {x} />
{/if}
{#if y > 0}
<Child {x} />
{/if}

@ -0,0 +1,3 @@
import { test } from '../../test';
export default test({ compileOptions: { experimental: { async: true } } });

@ -0,0 +1,201 @@
import 'svelte/internal/disclose-version';
import 'svelte/internal/flags/async';
import * as $ from 'svelte/internal/client';
var root = $.from_html(`<!> <!> <!> <!> <!>`, 1);
export default function Async_if_chain($$anchor) {
function complex1() {
return 1;
}
let foo = true;
var blocking;
var $$promises = $.run([async () => blocking = await $.async_derived(() => foo)]);
var fragment = root();
var node = $.first_child(fragment);
$.async(node, [$$promises[0]], void 0, (node) => {
var consequent = ($$anchor) => {
var text = $.text('foo');
$.append($$anchor, text);
};
var consequent_1 = ($$anchor) => {
var text_1 = $.text('bar');
$.append($$anchor, text_1);
};
var alternate = ($$anchor) => {
var text_2 = $.text('else');
$.append($$anchor, text_2);
};
$.if(node, ($$render) => {
if (foo) $$render(consequent); else if (bar) $$render(consequent_1, 1); else $$render(alternate, false);
});
});
var node_1 = $.sibling(node, 2);
$.async(node_1, [$$promises[0]], [() => foo], (node_1, $$condition) => {
var consequent_2 = ($$anchor) => {
var text_3 = $.text('foo');
$.append($$anchor, text_3);
};
var consequent_3 = ($$anchor) => {
var text_4 = $.text('bar');
$.append($$anchor, text_4);
};
var alternate_2 = ($$anchor) => {
var fragment_1 = $.comment();
var node_2 = $.first_child(fragment_1);
$.async(node_2, [], [() => baz], (node_2, $$condition) => {
var consequent_4 = ($$anchor) => {
var text_5 = $.text('baz');
$.append($$anchor, text_5);
};
var alternate_1 = ($$anchor) => {
var text_6 = $.text('else');
$.append($$anchor, text_6);
};
$.if(
node_2,
($$render) => {
if ($.get($$condition)) $$render(consequent_4); else $$render(alternate_1, false);
},
true
);
});
$.append($$anchor, fragment_1);
};
$.if(node_1, ($$render) => {
if ($.get($$condition)) $$render(consequent_2); else if (bar) $$render(consequent_3, 1); else $$render(alternate_2, false);
});
});
var node_3 = $.sibling(node_1, 2);
$.async(node_3, [$$promises[0]], [async () => (await $.save(foo))() > 10], (node_3, $$condition) => {
var consequent_5 = ($$anchor) => {
var text_7 = $.text('foo');
$.append($$anchor, text_7);
};
var consequent_6 = ($$anchor) => {
var text_8 = $.text('bar');
$.append($$anchor, text_8);
};
var alternate_4 = ($$anchor) => {
var fragment_2 = $.comment();
var node_4 = $.first_child(fragment_2);
$.async(node_4, [$$promises[0]], [async () => (await $.save(foo))() > 5], (node_4, $$condition) => {
var consequent_7 = ($$anchor) => {
var text_9 = $.text('baz');
$.append($$anchor, text_9);
};
var alternate_3 = ($$anchor) => {
var text_10 = $.text('else');
$.append($$anchor, text_10);
};
$.if(
node_4,
($$render) => {
if ($.get($$condition)) $$render(consequent_7); else $$render(alternate_3, false);
},
true
);
});
$.append($$anchor, fragment_2);
};
$.if(node_3, ($$render) => {
if ($.get($$condition)) $$render(consequent_5); else if (bar) $$render(consequent_6, 1); else $$render(alternate_4, false);
});
});
var node_5 = $.sibling(node_3, 2);
{
var consequent_8 = ($$anchor) => {
var text_11 = $.text('foo');
$.append($$anchor, text_11);
};
var consequent_9 = ($$anchor) => {
var text_12 = $.text('bar');
$.append($$anchor, text_12);
};
var consequent_10 = ($$anchor) => {
var text_13 = $.text('baz');
$.append($$anchor, text_13);
};
var d = $.derived(() => complex1() * complex2 > 100);
var alternate_5 = ($$anchor) => {
var text_14 = $.text('else');
$.append($$anchor, text_14);
};
$.if(node_5, ($$render) => {
if (simple1) $$render(consequent_8); else if (simple2 > 10) $$render(consequent_9, 1); else if ($.get(d)) $$render(consequent_10, 2); else $$render(alternate_5, false);
});
}
var node_6 = $.sibling(node_5, 2);
$.async(node_6, [$$promises[0]], void 0, (node_6) => {
var consequent_11 = ($$anchor) => {
var text_15 = $.text('foo');
$.append($$anchor, text_15);
};
var consequent_12 = ($$anchor) => {
var text_16 = $.text('bar');
$.append($$anchor, text_16);
};
var alternate_6 = ($$anchor) => {
var text_17 = $.text('else');
$.append($$anchor, text_17);
};
$.if(node_6, ($$render) => {
if ($.get(blocking) > 10) $$render(consequent_11); else if ($.get(blocking) > 5) $$render(consequent_12, 1); else $$render(alternate_6, false);
});
});
$.append($$anchor, fragment);
}

@ -0,0 +1,110 @@
import 'svelte/internal/flags/async';
import * as $ from 'svelte/internal/server';
export default function Async_if_chain($$renderer) {
function complex1() {
return 1;
}
let foo = true;
var blocking;
var $$promises = $$renderer.run([async () => blocking = await foo]);
$$renderer.async_block([$$promises[0]], ($$renderer) => {
if (foo) {
$$renderer.push('<!--[-->');
$$renderer.push(`foo`);
} else if (bar) {
$$renderer.push('<!--[1-->');
$$renderer.push(`bar`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push(`else`);
}
});
$$renderer.push(`<!--]--> `);
$$renderer.async_block([$$promises[0]], async ($$renderer) => {
if ((await $.save(foo))()) {
$$renderer.push('<!--[-->');
$$renderer.push(`foo`);
} else if (bar) {
$$renderer.push('<!--[1-->');
$$renderer.push(`bar`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.child_block(async ($$renderer) => {
if ((await $.save(baz))()) {
$$renderer.push('<!--[-->');
$$renderer.push(`baz`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push(`else`);
}
});
$$renderer.push(`<!--]-->`);
}
});
$$renderer.push(`<!--]--> `);
$$renderer.async_block([$$promises[0]], async ($$renderer) => {
if ((await $.save(foo))() > 10) {
$$renderer.push('<!--[-->');
$$renderer.push(`foo`);
} else if (bar) {
$$renderer.push('<!--[1-->');
$$renderer.push(`bar`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.async_block([$$promises[0]], async ($$renderer) => {
if ((await $.save(foo))() > 5) {
$$renderer.push('<!--[-->');
$$renderer.push(`baz`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push(`else`);
}
});
$$renderer.push(`<!--]-->`);
}
});
$$renderer.push(`<!--]--> `);
if (simple1) {
$$renderer.push('<!--[-->');
$$renderer.push(`foo`);
} else if (simple2 > 10) {
$$renderer.push('<!--[1-->');
$$renderer.push(`bar`);
} else if (complex1() * complex2 > 100) {
$$renderer.push('<!--[2-->');
$$renderer.push(`baz`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push(`else`);
}
$$renderer.push(`<!--]--> `);
$$renderer.async_block([$$promises[0]], ($$renderer) => {
if (blocking > 10) {
$$renderer.push('<!--[-->');
$$renderer.push(`foo`);
} else if (blocking > 5) {
$$renderer.push('<!--[1-->');
$$renderer.push(`bar`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push(`else`);
}
});
$$renderer.push(`<!--]-->`);
}

@ -0,0 +1,59 @@
<script>
function complex1() {
return 1;
}
let foo = $state(true);
let blocking = $derived(await foo);
</script>
<!-- simple chain - should have no nested $.if() -->
{#if foo}
foo
{:else if bar}
bar
{:else}
else
{/if}
<!-- simple chain with await expressions - should have $.if() at each await expression -->
{#if await foo}
foo
{:else if bar}
bar
{:else if await baz}
baz
{:else}
else
{/if}
<!-- simple chain with await expressions #2 - should have $.if() at each await expression (ideally we can detect that await foo is unnecessary to await multiple times and this is one $.if()) -->
{#if await foo > 10}
foo
{:else if bar}
bar
{:else if await foo > 5}
baz
{:else}
else
{/if}
<!-- simple chain with some expressions that cause a $.derived - should be one $.if() -->
{#if simple1}
foo
{:else if simple2 > 10}
bar
{:else if complex1() * complex2 > 100}
baz
{:else}
else
{/if}
<!-- simple chain with blocking expressions - should be one $.if() -->
{#if blocking > 10}
foo
{:else if blocking > 5}
bar
{:else}
else
{/if}
Loading…
Cancel
Save