pull/17004/head
Rich Harris 5 days ago
commit 54cd91f055

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: keep batches alive until all async work is complete

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: don't preserve reactivity context across function boundaries

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: ensure guards (eg. if, each, key) run before their contents

@ -306,7 +306,7 @@ export function analyze_module(source, options) {
fragment: null, fragment: null,
parent_element: null, parent_element: null,
reactive_statement: null, reactive_statement: null,
in_derived: false derived_function_depth: -1
}, },
visitors visitors
); );
@ -703,7 +703,7 @@ export function analyze_component(root, source, options) {
state_fields: new Map(), state_fields: new Map(),
function_depth: scope.function_depth, function_depth: scope.function_depth,
reactive_statement: null, reactive_statement: null,
in_derived: false derived_function_depth: -1
}; };
walk(/** @type {AST.SvelteNode} */ (ast), state, visitors); walk(/** @type {AST.SvelteNode} */ (ast), state, visitors);
@ -771,7 +771,7 @@ export function analyze_component(root, source, options) {
expression: null, expression: null,
state_fields: new Map(), state_fields: new Map(),
function_depth: scope.function_depth, function_depth: scope.function_depth,
in_derived: false derived_function_depth: -1
}; };
walk(/** @type {AST.SvelteNode} */ (ast), state, visitors); walk(/** @type {AST.SvelteNode} */ (ast), state, visitors);

@ -29,9 +29,9 @@ export interface AnalysisState {
reactive_statement: null | ReactiveStatement; reactive_statement: null | ReactiveStatement;
/** /**
* True if we're directly inside a `$derived(...)` expression (but not `$derived.by(...)`) * Set when we're inside a `$derived(...)` expression (but not `$derived.by(...)`) or `@const`
*/ */
in_derived: boolean; derived_function_depth: number;
} }
export type Context<State extends AnalysisState = AnalysisState> = import('zimmerframe').Context< export type Context<State extends AnalysisState = AnalysisState> = import('zimmerframe').Context<

@ -15,7 +15,10 @@ export function AwaitExpression(node, context) {
// b) awaits that precede other expressions in template or `$derived(...)` // b) awaits that precede other expressions in template or `$derived(...)`
if ( if (
tla || tla ||
(is_reactive_expression(context.path, context.state.in_derived) && (is_reactive_expression(
context.path,
context.state.derived_function_depth === context.state.function_depth
) &&
!is_last_evaluated_expression(context.path, node)) !is_last_evaluated_expression(context.path, node))
) { ) {
context.state.analysis.pickled_awaits.add(node); context.state.analysis.pickled_awaits.add(node);
@ -53,9 +56,7 @@ export function AwaitExpression(node, context) {
* @param {boolean} in_derived * @param {boolean} in_derived
*/ */
export function is_reactive_expression(path, in_derived) { export function is_reactive_expression(path, in_derived) {
if (in_derived) { if (in_derived) return true;
return true;
}
let i = path.length; let i = path.length;
@ -67,6 +68,7 @@ export function is_reactive_expression(path, in_derived) {
parent.type === 'FunctionExpression' || parent.type === 'FunctionExpression' ||
parent.type === 'FunctionDeclaration' parent.type === 'FunctionDeclaration'
) { ) {
// No reactive expression found between function and await
return false; return false;
} }
@ -83,11 +85,16 @@ export function is_reactive_expression(path, in_derived) {
* @param {AST.SvelteNode[]} path * @param {AST.SvelteNode[]} path
* @param {Expression | SpreadElement | Property} node * @param {Expression | SpreadElement | Property} node
*/ */
export function is_last_evaluated_expression(path, node) { function is_last_evaluated_expression(path, node) {
let i = path.length; let i = path.length;
while (i--) { while (i--) {
const parent = /** @type {Expression | Property | SpreadElement} */ (path[i]); const parent = path[i];
if (parent.type === 'ConstTag') {
// {@const ...} tags are treated as deriveds and its contents should all get the preserve-reactivity treatment
return false;
}
// @ts-expect-error we could probably use a neater/more robust mechanism // @ts-expect-error we could probably use a neater/more robust mechanism
if (parent.metadata) { if (parent.metadata) {

@ -248,7 +248,7 @@ export function CallExpression(node, context) {
context.next({ context.next({
...context.state, ...context.state,
function_depth: context.state.function_depth + 1, function_depth: context.state.function_depth + 1,
in_derived: true, derived_function_depth: context.state.function_depth + 1,
expression expression
}); });

@ -38,6 +38,8 @@ export function ConstTag(node, context) {
context.visit(declaration.init, { context.visit(declaration.init, {
...context.state, ...context.state,
expression: node.metadata.expression, expression: node.metadata.expression,
in_derived: true // We're treating this like a $derived under the hood
function_depth: context.state.function_depth + 1,
derived_function_depth: context.state.function_depth + 1
}); });
} }

@ -64,12 +64,6 @@ export function VariableDeclarator(node, context) {
} }
} }
if (rune === '$derived') {
context.visit(node.id);
context.visit(/** @type {Expression} */ (node.init), { ...context.state, in_derived: true });
return;
}
if (rune === '$props') { if (rune === '$props') {
if (node.id.type !== 'ObjectPattern' && node.id.type !== 'Identifier') { if (node.id.type !== 'ObjectPattern' && node.id.type !== 'Identifier') {
e.props_invalid_identifier(node); e.props_invalid_identifier(node);

@ -1,5 +1,5 @@
/** @import { Effect, TemplateNode, Value } from '#client' */ /** @import { Effect, TemplateNode, Value } from '#client' */
import { DESTROYED } from '#client/constants'; import { DESTROYED, STALE_REACTION } from '#client/constants';
import { DEV } from 'esm-env'; import { DEV } from 'esm-env';
import { import {
component_context, component_context,

@ -29,7 +29,14 @@ import * as e from '../errors.js';
import { flush_tasks, queue_micro_task } from '../dom/task.js'; import { flush_tasks, queue_micro_task } from '../dom/task.js';
import { DEV } from 'esm-env'; import { DEV } from 'esm-env';
import { invoke_error_boundary } from '../error-handling.js'; import { invoke_error_boundary } from '../error-handling.js';
import { old_values, source, update } from './sources.js'; import {
flush_inspect_effects,
inspect_effects,
old_values,
set_inspect_effects,
source,
update
} from './sources.js';
import { inspect_effect, unlink_effect } from './effects.js'; import { inspect_effect, unlink_effect } from './effects.js';
/** /**
@ -76,6 +83,8 @@ let is_flushing = false;
export let is_flushing_sync = false; export let is_flushing_sync = false;
export class Batch { export class Batch {
committed = false;
/** /**
* The current values of any sources that are updated in this batch * The current values of any sources that are updated in this batch
* They keys of this map are identical to `this.#previous` * They keys of this map are identical to `this.#previous`
@ -88,7 +97,7 @@ export class Batch {
* They keys of this map are identical to `this.#current` * They keys of this map are identical to `this.#current`
* @type {Map<Source, any>} * @type {Map<Source, any>}
*/ */
#previous = new Map(); previous = new Map();
/** /**
* When the batch is committed (and the DOM is updated), we need to remove old branches * When the batch is committed (and the DOM is updated), we need to remove old branches
@ -166,16 +175,7 @@ export class Batch {
this.#traverse_effect_tree(root, target); this.#traverse_effect_tree(root, target);
} }
// if there is no outstanding async work, commit this.#resolve();
if (this.#pending === 0) {
if (this.is_fork) {
this.#fork_deferred?.resolve();
} else {
// commit before flushing effects, since that may result in
// another batch being created
this.#commit();
}
}
if (this.#blocking_pending > 0 || this.is_fork) { if (this.#blocking_pending > 0 || this.is_fork) {
this.#defer_effects(target.effects); this.#defer_effects(target.effects);
@ -291,8 +291,8 @@ export class Batch {
* @param {any} value * @param {any} value
*/ */
capture(source, value) { capture(source, value) {
if (!this.#previous.has(source)) { if (!this.previous.has(source)) {
this.#previous.set(source, value); this.previous.set(source, value);
} }
this.current.set(source, source.v); this.current.set(source, source.v);
@ -334,16 +334,29 @@ export class Batch {
} }
} }
/** #resolve() {
* Append and remove branches to/from the DOM if (this.#blocking_pending === 0 && !this.is_fork) {
*/ // append/remove branches
for (const fn of this.#callbacks) fn();
this.#callbacks.clear();
}
if (this.#pending === 0) {
if (this.is_fork) {
this.#fork_deferred?.resolve();
} else {
this.#commit();
}
}
}
#commit() { #commit() {
// If there are other pending batches, they now need to be 'rebased' — // If there are other pending batches, they now need to be 'rebased' —
// in other words, we re-run block/async effects with the newly // in other words, we re-run block/async effects with the newly
// committed state, unless the batch in question has a more // committed state, unless the batch in question has a more
// recent value for a given source // recent value for a given source
if (batches.size > 1) { if (batches.size > 1) {
this.#previous.clear(); this.previous.clear();
var previous_batch_values = batch_values; var previous_batch_values = batch_values;
var is_earlier = true; var is_earlier = true;
@ -412,6 +425,7 @@ export class Batch {
batch_values = previous_batch_values; batch_values = previous_batch_values;
} }
this.committed = true;
batches.delete(this); batches.delete(this);
this.#deferred?.resolve(); this.#deferred?.resolve();
@ -503,7 +517,7 @@ export class Batch {
for (const batch of batches) { for (const batch of batches) {
if (batch === this) continue; if (batch === this) continue;
for (const [source, previous] of batch.#previous) { for (const [source, previous] of batch.previous) {
if (!batch_values.has(source)) { if (!batch_values.has(source)) {
batch_values.set(source, previous); batch_values.set(source, previous);
} }
@ -625,7 +639,7 @@ function infinite_loop_guard() {
} }
} }
/** @type {Effect[] | null} */ /** @type {Set<Effect> | null} */
export let eager_block_effects = null; export let eager_block_effects = null;
/** /**
@ -642,7 +656,7 @@ function flush_queued_effects(effects) {
var effect = effects[i++]; var effect = effects[i++];
if ((effect.f & (DESTROYED | INERT)) === 0 && is_dirty(effect)) { if ((effect.f & (DESTROYED | INERT)) === 0 && is_dirty(effect)) {
eager_block_effects = []; eager_block_effects = new Set();
update_effect(effect); update_effect(effect);
@ -665,15 +679,34 @@ function flush_queued_effects(effects) {
// If update_effect() has a flushSync() in it, we may have flushed another flush_queued_effects(), // If update_effect() has a flushSync() in it, we may have flushed another flush_queued_effects(),
// which already handled this logic and did set eager_block_effects to null. // which already handled this logic and did set eager_block_effects to null.
if (eager_block_effects?.length > 0) { if (eager_block_effects?.size > 0) {
// TODO this feels incorrect! it gets the tests passing
old_values.clear(); old_values.clear();
for (const e of eager_block_effects) { for (const e of eager_block_effects) {
update_effect(e); // Skip eager effects that have already been unmounted
if ((e.f & (DESTROYED | INERT)) !== 0) continue;
// Run effects in order from ancestor to descendant, else we could run into nullpointers
/** @type {Effect[]} */
const ordered_effects = [e];
let ancestor = e.parent;
while (ancestor !== null) {
if (eager_block_effects.has(ancestor)) {
eager_block_effects.delete(ancestor);
ordered_effects.push(ancestor);
}
ancestor = ancestor.parent;
}
for (let j = ordered_effects.length - 1; j >= 0; j--) {
const e = ordered_effects[j];
// Skip eager effects that have already been unmounted
if ((e.f & (DESTROYED | INERT)) !== 0) continue;
update_effect(e);
}
} }
eager_block_effects = []; eager_block_effects.clear();
} }
} }
} }
@ -828,17 +861,47 @@ export function fork(fn) {
const batch = Batch.ensure(); const batch = Batch.ensure();
batch.is_fork = true; batch.is_fork = true;
fn(); flushSync(fn);
await batch.fork_settled(); const deferred_inspect_effects = inspect_effects;
// TODO revert state changes // revert state changes
for (const [source, value] of batch.previous) {
source.v = value;
}
await batch.fork_settled();
fulfil({ fulfil({
commit: () => { commit: () => {
// TODO reapply state changes if (!batches.has(batch)) {
batch.is_fork = false; throw new Error('Cannot commit this batch'); // TODO better error
batch.activate(); }
batch.revive();
// delete all other forks
for (const b of batches) {
if (b !== batch && b.is_fork) {
batches.delete(b);
}
}
for (const [source, value] of batch.current) {
source.v = value;
}
const previous_inspect_effects = inspect_effects;
try {
if (DEV && deferred_inspect_effects.size > 0) {
set_inspect_effects(deferred_inspect_effects);
flush_inspect_effects();
}
batch.is_fork = false;
batch.activate();
batch.revive();
} finally {
set_inspect_effects(previous_inspect_effects);
}
}, },
discard: () => { discard: () => {
batches.delete(batch); batches.delete(batch);

@ -127,7 +127,17 @@ export function async_derived(fn, location) {
// If this code is changed at some point, make sure to still access the then property // If this code is changed at some point, make sure to still access the then property
// of fn() to read any signals it might access, so that we track them as dependencies. // of fn() to read any signals it might access, so that we track them as dependencies.
// We call `unset_context` to undo any `save` calls that happen inside `fn()` // We call `unset_context` to undo any `save` calls that happen inside `fn()`
Promise.resolve(fn()).then(d.resolve, d.reject).then(unset_context); Promise.resolve(fn())
.then(d.resolve, d.reject)
.then(() => {
if (batch === current_batch && batch.committed) {
// if the batch was rejected as stale, we need to cleanup
// after any `$.save(...)` calls inside `fn()`
batch.deactivate();
}
unset_context();
});
} catch (error) { } catch (error) {
d.reject(error); d.reject(error);
unset_context(); unset_context();

@ -234,7 +234,7 @@ export function internal_set(source, value) {
} }
} }
if (DEV && inspect_effects.size > 0 && !inspect_effects_deferred) { if (DEV && !batch.is_fork && inspect_effects.size > 0 && !inspect_effects_deferred) {
flush_inspect_effects(); flush_inspect_effects();
} }
} }
@ -336,7 +336,7 @@ function mark_reactions(signal, status) {
} else if (not_dirty) { } else if (not_dirty) {
if ((flags & BLOCK_EFFECT) !== 0) { if ((flags & BLOCK_EFFECT) !== 0) {
if (eager_block_effects !== null) { if (eager_block_effects !== null) {
eager_block_effects.push(/** @type {Effect} */ (reaction)); eager_block_effects.add(/** @type {Effect} */ (reaction));
} }
} }

@ -0,0 +1,63 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target }) {
const [increment, shift] = target.querySelectorAll('button');
shift.click();
await tick();
shift.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>0</button>
<button>shift</button>
<p>even</p>
<p>0</p>
`
);
increment.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>1</button>
<button>shift</button>
<p>even</p>
<p>0</p>
`
);
shift.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>1</button>
<button>shift</button>
<p>odd</p>
<p>loading...</p>
`
);
shift.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>1</button>
<button>shift</button>
<p>odd</p>
<p>1</p>
`
);
}
});

@ -0,0 +1,36 @@
<script>
let resolvers = [];
function push(value) {
const { promise, resolve } = Promise.withResolvers();
resolvers.push(() => resolve(value));
return promise;
}
let count = $state(0);
</script>
<button onclick={() => count += 1}>{$state.eager(count)}</button>
<button onclick={() => resolvers.shift()?.()}>shift</button>
<svelte:boundary>
{#if await push(count) % 2 === 0}
<p>even</p>
{:else}
<p>odd</p>
{/if}
{#key count}
<svelte:boundary>
<p>{await push(count)}</p>
{#snippet pending()}
<p>loading...</p>
{/snippet}
</svelte:boundary>
{/key}
{#snippet pending()}
<p>loading...</p>
{/snippet}
</svelte:boundary>

@ -1,7 +1,11 @@
<script> <script>
$effect(() => {
console.log('before');
});
await 1; await 1;
$effect(() => { $effect(() => {
console.log('hello'); console.log('after');
}); });
</script> </script>

@ -3,7 +3,8 @@ import { test } from '../../test';
export default test({ export default test({
async test({ assert, logs }) { async test({ assert, logs }) {
assert.deepEqual(logs, []);
await tick(); await tick();
assert.deepEqual(logs, ['hello']); assert.deepEqual(logs, ['before', 'after']);
} }
}); });

@ -3,23 +3,27 @@ import { test } from '../../test';
export default test({ export default test({
async test({ assert, target }) { async test({ assert, target }) {
// We gotta wait a bit more in this test because of the macrotasks in App.svelte // We gotta wait a bit more in this test because of the macrotasks in App.svelte
function macrotask(t = 3) { function sleep(t = 50) {
return new Promise((r) => setTimeout(r, t)); return new Promise((r) => setTimeout(r, t));
} }
await macrotask(); await sleep();
assert.htmlEqual(target.innerHTML, '<input> 1 | '); assert.htmlEqual(target.innerHTML, '<input> 1 | ');
const [input] = target.querySelectorAll('input'); const [input] = target.querySelectorAll('input');
input.value = '1'; input.value = '1';
input.dispatchEvent(new Event('input', { bubbles: true })); input.dispatchEvent(new Event('input', { bubbles: true }));
await macrotask(); await sleep();
assert.htmlEqual(target.innerHTML, '<input> 1 | '); assert.htmlEqual(target.innerHTML, '<input> 1 | ');
input.value = '12'; input.value = '12';
input.dispatchEvent(new Event('input', { bubbles: true })); input.dispatchEvent(new Event('input', { bubbles: true }));
await macrotask(6); await sleep();
assert.htmlEqual(target.innerHTML, '<input> 3 | 12'); assert.htmlEqual(target.innerHTML, '<input> 3 | 12');
input.value = '';
input.dispatchEvent(new Event('input', { bubbles: true }));
await sleep();
assert.htmlEqual(target.innerHTML, '<input> 4 | ');
} }
}); });

@ -1,28 +1,32 @@
<script> <script>
let count = $state(0); let count = $state(0);
let value = $state(''); let value = $state('');
let prev;
function asd(v) { let resolver;
const r = Promise.withResolvers();
if (prev || v === '') { function asd(v) {
Promise.resolve().then(async () => { let r = Promise.withResolvers();
count++;
r.resolve(v); function update_and_resolve(){
await new Promise(r => setTimeout(r, 0)); count++;
// TODO with a microtask like below it still throws a mutation error r.resolve(v);
// await Promise.resolve();
prev?.resolve();
});
} else {
prev = Promise.withResolvers();
prev.promise.then(() => {
count++;
r.resolve(v)
});
} }
// make sure the second promise resolve before the first one
if(resolver){
new Promise(r => {
setTimeout(r);
}).then(update_and_resolve).then(()=>{
setTimeout(()=>{
resolver();
resolver = null;
});
});
}else if(v){
resolver = update_and_resolve;
}else{
Promise.resolve().then(update_and_resolve);
}
return r.promise; return r.promise;
} }

@ -0,0 +1,20 @@
import { test } from '../../test';
import { flushSync } from 'svelte';
export default test({
mode: ['client'],
async test({ target, assert, logs }) {
const button = target.querySelector('button');
button?.click();
flushSync();
button?.click();
flushSync();
button?.click();
flushSync();
button?.click();
flushSync();
assert.deepEqual(logs, ['two', 'one', 'two', 'one', 'two']);
}
});

@ -0,0 +1,18 @@
<script lang="ts">
let b = $state(false);
let v = $state("two");
$effect(() => {
v = b ? "one" : "two";
})
</script>
<button onclick={() => b = !b}>Trigger</button>
{#if v === "one"}
<div>if1 matched! {console.log('one')}</div>
{:else if v === "two"}
<div>if2 matched! {console.log('two')}</div>
{:else}
<div>nothing matched {console.log('else')}</div>
{/if}

@ -0,0 +1,13 @@
import { test } from '../../test';
import { flushSync } from 'svelte';
export default test({
mode: ['client'],
async test({ target, assert }) {
const button = target.querySelector('button');
flushSync(() => button?.click());
assert.equal(target.textContent?.trim(), 'Trigger');
}
});

@ -0,0 +1,18 @@
<script>
let centerRow = $state({ nested: { optional: 2, required: 3 } });
let someChange = $state(false);
$effect(() => {
if (someChange) centerRow = undefined;
});
</script>
{#if centerRow?.nested}
{#if centerRow?.nested?.optional != undefined && centerRow.nested.optional > 0}
op: {centerRow.nested.optional}<br />
{:else}
req: {centerRow.nested.required}<br />
{/if}
{/if}
<button onclick={() => (someChange = true)}>Trigger</button>

@ -0,0 +1,6 @@
<script>
let { p } = $props();
$effect.pre(() => {
console.log('running ' + p)
})
</script>

@ -0,0 +1,13 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
mode: ['client'],
async test({ assert, target, logs }) {
const button = target.querySelector('button');
button?.click();
flushSync();
assert.deepEqual(logs, ['pre', 'running b', 'pre', 'pre']);
}
});

@ -0,0 +1,18 @@
<script>
import Component from './Component.svelte';
let p = $state('b');
$effect.pre(() => {
console.log('pre')
if (p === 'a') p = null;
})
</script>
{#if p || !p}
{#if p}
<Component {p} />
{/if}
{/if}
<button onclick={() => p = 'a'}>a</button>

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

@ -0,0 +1,52 @@
import 'svelte/internal/disclose-version';
import 'svelte/internal/flags/async';
import * as $ from 'svelte/internal/client';
export default function Async_in_derived($$anchor, $$props) {
$.push($$props, true);
$.async_body($$anchor, async ($$anchor) => {
let yes1 = (await $.save($.async_derived(async () => (await $.save(1))())))();
let yes2 = (await $.save($.async_derived(async () => foo((await $.save(1))()))))();
let no1 = $.derived(async () => {
return await 1;
});
let no2 = $.derived(() => async () => {
return await 1;
});
if ($.aborted()) return;
var fragment = $.comment();
var node = $.first_child(fragment);
{
var consequent = ($$anchor) => {
$.async_body($$anchor, async ($$anchor) => {
const yes1 = (await $.save($.async_derived(async () => (await $.save(1))())))();
const yes2 = (await $.save($.async_derived(async () => foo((await $.save(1))()))))();
const no1 = $.derived(() => (async () => {
return await 1;
})());
const no2 = $.derived(() => (async () => {
return await 1;
})());
if ($.aborted()) return;
});
};
$.if(node, ($$render) => {
if (true) $$render(consequent);
});
}
$.append($$anchor, fragment);
});
$.pop();
}

@ -0,0 +1,40 @@
import 'svelte/internal/flags/async';
import * as $ from 'svelte/internal/server';
export default function Async_in_derived($$renderer, $$props) {
$$renderer.component(($$renderer) => {
$$renderer.async(async ($$renderer) => {
let yes1 = (await $.save(1))();
let yes2 = foo((await $.save(1))());
let no1 = (async () => {
return await 1;
})();
let no2 = async () => {
return await 1;
};
$$renderer.async(async ($$renderer) => {
if (true) {
$$renderer.push('<!--[-->');
const yes1 = (await $.save(1))();
const yes2 = foo((await $.save(1))());
const no1 = (async () => {
return await 1;
})();
const no2 = (async () => {
return await 1;
})();
} else {
$$renderer.push('<!--[!-->');
}
});
$$renderer.push(`<!--]-->`);
});
});
}

@ -0,0 +1,21 @@
<script>
let yes1 = $derived(await 1);
let yes2 = $derived(foo(await 1));
let no1 = $derived.by(async () => {
return await 1;
});
let no2 = $derived(async () => {
return await 1;
});
</script>
{#if true}
{@const yes1 = await 1}
{@const yes2 = foo(await 1)}
{@const no1 = (async () => {
return await 1;
})()}
{@const no2 = (async () => {
return await 1;
})()}
{/if}
Loading…
Cancel
Save