fix: ensure async block assigns correct nodes to effect (#18371)

If a block has a sole component child, we apply an optimization to not
need a template. But if that sole child is wrapped in `$.async`, it can
happens that the block's effect is assigned the wrong end node.

In the reproduction this happens because we have two components inside a
Child which is rendered only after an async block resolves. In
`$.append` we have logic to not reassign nodes when the active effect
didn't already run, which it has by the time we come across it (because
the async work had to resolve first; we added this in
https://github.com/sveltejs/svelte/pull/17120 to prevent the opposite,
nodes being "rewinded" to an earlier position) and so the second sibling
component's `$.append` will not set its end node to the effect. As a
result the end node is wrong (too early).

To fix this we assign the nodes in `$.async`. We could also use
`compareDocumentPosition` in `$.append` to only ever go forward, but
that feels a bit heavier performance-wise.

---------

Co-authored-by: paoloricciuti <ricciutipaolo@gmail.com>
Co-authored-by: Rich Harris <rich.harris@vercel.com>
pull/18357/head
Simon H 1 month ago committed by GitHub
parent e4bfc5f45a
commit bdb1a0f847
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: properly track effect end node for async sibling component

@ -9,6 +9,7 @@ import {
set_hydrating,
skip_nodes
} from '../hydration.js';
import { assign_nodes } from '../template.js';
/**
* @param {TemplateNode} node
@ -23,6 +24,7 @@ export function async(node, blockers = [], expressions = [], fn) {
if (was_hydrating) {
hydrate_next();
end = skip_nodes(false);
assign_nodes(node, end); // Necessary if this wraps the sole child of a block, else end marker can be wrong
}
if (expressions.length === 0 && blockers.every((b) => b.settled)) {

@ -72,7 +72,7 @@ const { test, run } = suite<HydrationTest>(async (config, cwd) => {
const target = window.document.body;
const head = window.document.head;
const rendered = render((await import(`${cwd}/_output/server/main.svelte.js`)).default, {
const rendered = await render((await import(`${cwd}/_output/server/main.svelte.js`)).default, {
props: config.server_props ?? config.props ?? {},
idPrefix: config?.id_prefix
});
@ -80,8 +80,8 @@ const { test, run } = suite<HydrationTest>(async (config, cwd) => {
const override = read(`${cwd}/_override.html`);
const override_head = read(`${cwd}/_override_head.html`);
fs.writeFileSync(`${cwd}/_output/body.html`, rendered.html + '\n');
target.innerHTML = override ?? rendered.html;
fs.writeFileSync(`${cwd}/_output/body.html`, rendered.body + '\n');
target.innerHTML = override ?? rendered.body;
if (rendered.head) {
fs.writeFileSync(`${cwd}/_output/head.html`, rendered.head + '\n');
@ -145,7 +145,7 @@ const { test, run } = suite<HydrationTest>(async (config, cwd) => {
flushSync();
const expected = read(`${cwd}/_expected.html`) ?? rendered.html;
const expected = read(`${cwd}/_expected.html`) ?? rendered.body;
assert_html_equal(target.innerHTML, expected);
if (rendered.head) {

@ -0,0 +1,7 @@
<script>
import Header from './Header.svelte';
import Footer from './Footer.svelte';
</script>
<Header />
<Footer />

@ -0,0 +1,38 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target }) {
await tick();
const [toggle] = target.querySelectorAll('button');
assert.htmlEqual(
target.innerHTML,
`
<button>toggle</button>
<header>header</header>
<footer>footer</footer>
`
);
toggle.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>toggle</button>
`
);
toggle.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>toggle</button>
<header>header</header>
<footer>footer</footer>
`
);
}
});

@ -0,0 +1,11 @@
<script>
import Child from './Child.svelte';
let show = $state(true);
</script>
<button onclick={() => (show = !show)}>toggle</button>
{#if show}
<Child gate={await Promise.resolve(true)} />
{/if}
Loading…
Cancel
Save