fix: don't override new current_batch (#18170)

This is a regression from #18117 - we moved `this.#commit()` higher up
but that means that `current_batch` could be nulled out / overridden
through `batch.activate/deactivate` / blocker runs inside `#commit()`.
Therefore restore the previous value afterwards. No changest because
#18117 is not released yet.
Fixes the other part of the failing SvelteKit `query.live` test.

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
pull/18184/head
Simon H 4 months ago committed by GitHub
parent d4c5a91735
commit 5e054574db
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -92,6 +92,9 @@ let uid = 1;
export class Batch { export class Batch {
id = uid++; id = uid++;
/** True as soon as `#process()` was called */
#started = false;
/** /**
* The current values of any signals that are updated in this batch. * The current values of any signals that are updated in this batch.
* Tuple format: [value, is_derived] (note: is_derived is false for deriveds, too, if they were overridden via assignment) * Tuple format: [value, is_derived] (note: is_derived is false for deriveds, too, if they were overridden via assignment)
@ -255,6 +258,8 @@ export class Batch {
} }
#process() { #process() {
this.#started = true;
if (flush_count++ > 1000) { if (flush_count++ > 1000) {
batches.delete(this); batches.delete(this);
infinite_loop_guard(); infinite_loop_guard();
@ -342,6 +347,8 @@ export class Batch {
this.#deferred?.resolve(); this.#deferred?.resolve();
} }
var next_batch = /** @type {Batch | null} */ (/** @type {unknown} */ (current_batch));
// Order matters here - we need to commit and THEN continue flushing new batches, not the other way around, // Order matters here - we need to commit and THEN continue flushing new batches, not the other way around,
// else we could start flushing a new batch and then, if it has pending work, rebase it right afterwards, which is wrong. // else we could start flushing a new batch and then, if it has pending work, rebase it right afterwards, which is wrong.
// In sync mode flushSync can cause #commit to wrongfully think that there needs to be a rebase, so we only do it in async mode // In sync mode flushSync can cause #commit to wrongfully think that there needs to be a rebase, so we only do it in async mode
@ -350,8 +357,6 @@ export class Batch {
this.#commit(); this.#commit();
} }
var next_batch = /** @type {Batch | null} */ (/** @type {unknown} */ (current_batch));
// Edge case: During traversal new branches might create effects that run immediately and set state, // Edge case: During traversal new branches might create effects that run immediately and set state,
// causing an effect and therefore a root to be scheduled again. We need to traverse the current batch // causing an effect and therefore a root to be scheduled again. We need to traverse the current batch
// once more in that case - most of the time this will just clean up dirty branches. // once more in that case - most of the time this will just clean up dirty branches.
@ -537,6 +542,8 @@ export class Batch {
sources.push(source); sources.push(source);
} }
if (!batch.#started) continue;
// Re-run async/block effects that depend on distinct values changed in both batches // Re-run async/block effects that depend on distinct values changed in both batches
var others = [...batch.current.keys()].filter((s) => !this.current.has(s)); var others = [...batch.current.keys()].filter((s) => !this.current.has(s));
@ -722,7 +729,7 @@ export class Batch {
if (!is_flushing_sync) { if (!is_flushing_sync) {
queue_micro_task(() => { queue_micro_task(() => {
if (!batches.has(batch) || batch.#pending.size > 0) { if (batch.#started) {
// a flushSync happened in the meantime // a flushSync happened in the meantime
return; return;
} }

@ -60,6 +60,8 @@ export interface RuntimeTest<Props extends Record<string, any> = Record<string,
id_prefix?: string; id_prefix?: string;
before_test?: () => void; before_test?: () => void;
after_test?: () => void; after_test?: () => void;
/** If true, flushSync() will not be called before invoking test() */
skip_initial_flushSync?: boolean;
test?: (args: { test?: (args: {
variant: 'dom' | 'hydrate'; variant: 'dom' | 'hydrate';
assert: Assert; assert: Assert;
@ -505,7 +507,7 @@ async function run_test_variant(
try { try {
if (config.test) { if (config.test) {
flushSync(); if (!config.skip_initial_flushSync) flushSync();
if (variant === 'hydrate' && cwd.includes('async-')) { if (variant === 'hydrate' && cwd.includes('async-')) {
// wait for pending boundaries to render // wait for pending boundaries to render
@ -543,7 +545,7 @@ async function run_test_variant(
} }
} finally { } finally {
if (runes) { if (runes) {
unmount(instance); await unmount(instance);
} else { } else {
instance.$destroy(); instance.$destroy();
} }

@ -0,0 +1,19 @@
import { tick } from 'svelte';
import { test } from '../../test';
// Tests that batch.#commit() does not null out a potentially new current_batch
export default test({
skip_initial_flushSync: true, // test that the initial batch is flushed without an explicit flushSync() call
async test({ assert, target }) {
await tick();
const [button] = target.querySelectorAll('button');
const [updates] = target.querySelectorAll('p');
assert.htmlEqual(updates.innerHTML, 'false');
button.click();
await tick();
assert.htmlEqual(updates.innerHTML, 'true');
}
});

@ -0,0 +1,30 @@
<script>
let count = $state(-1);
let payload = $state(false);
let updated = $state(false);
$effect(() => {
if (payload) {
updated = true;
}
});
function update() {
count = 0;
queueMicrotask(() => {
payload = true;
});
}
</script>
<button onclick={update}>update</button>
<p>{updated}</p>
<svelte:boundary>
{await new Promise(() => {})}
{#snippet pending()}
<p>pending</p>
{/snippet}
</svelte:boundary>
Loading…
Cancel
Save