fix: keep input in sync when binding updated via effect

Fixes #16413

The previous fix was insufficient as it didn't account for effects running as a result of a change, which is executed in a different batch. Instead we now set a boolean that is true while an async branch is flushed.
binding-fix-2
Simon Holthausen 1 month ago
parent ce4a99ed6d
commit bf7de3a56b

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: keep input in sync when binding updated via effect

@ -1,4 +1,3 @@
/** @import { Batch } from '../../../reactivity/batch.js' */
import { DEV } from 'esm-env';
import { render_effect, teardown } from '../../../reactivity/effects.js';
import { listen_to_event_and_reset_event } from './shared.js';
@ -19,8 +18,6 @@ import { current_batch } from '../../../reactivity/batch.js';
export function bind_value(input, get, set = get) {
var runes = is_runes();
var batches = new WeakSet();
listen_to_event_and_reset_event(input, 'input', (is_reset) => {
if (DEV && input.type === 'checkbox') {
// TODO should this happen in prod too?
@ -32,10 +29,6 @@ export function bind_value(input, get, set = get) {
value = is_numberlike_input(input) ? to_number(value) : value;
set(value);
if (current_batch !== null) {
batches.add(current_batch);
}
// In runes mode, respect any validation in accessors (doesn't apply in legacy mode,
// because we use mutable state which ensures the render effect always runs)
if (runes && value !== (value = get())) {
@ -62,10 +55,6 @@ export function bind_value(input, get, set = get) {
(untrack(get) == null && input.value)
) {
set(is_numberlike_input(input) ? to_number(input.value) : input.value);
if (current_batch !== null) {
batches.add(current_batch);
}
}
render_effect(() => {
@ -76,9 +65,9 @@ export function bind_value(input, get, set = get) {
var value = get();
if (input === document.activeElement && batches.has(/** @type {Batch} */ (current_batch))) {
// Never rewrite the contents of a focused input. We can get here if, for example,
// an update is deferred because of async work depending on the input:
if (input === document.activeElement && current_batch?.flushing_async) {
// Never rewrite the contents of a focused input when flushing async work.
// We can get here if, for example, an update is deferred because of async work depending on the input:
//
// <input bind:value={query}>
// <p>{await find(query)}</p>

@ -152,6 +152,11 @@ export class Batch {
*/
skipped_effects = new Set();
/**
* True while a batch that had asynchronous work (i.e. a pending count) is being flushed.
*/
flushing_async = false;
/**
*
* @param {Effect[]} root_effects
@ -412,6 +417,8 @@ export class Batch {
this.#pending -= 1;
if (this.#pending === 0) {
this.flushing_async = true;
for (const e of this.#render_effects) {
set_signal_status(e, DIRTY);
schedule_effect(e);
@ -431,6 +438,8 @@ export class Batch {
this.#effects = [];
this.flush();
this.flushing_async = true;
} else {
this.deactivate();
}

@ -0,0 +1,37 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target }) {
await new Promise((resolve) => setTimeout(resolve, 110));
const [input] = target.querySelectorAll('input');
assert.equal(input.value, 'a');
assert.htmlEqual(target.innerHTML, `<p>a</p><input />`);
flushSync(() => {
input.focus();
input.value = 'ab';
input.dispatchEvent(new InputEvent('input', { bubbles: true }));
});
await new Promise((resolve) => setTimeout(resolve, 50));
flushSync(() => {
input.focus();
input.value = 'abc';
input.dispatchEvent(new InputEvent('input', { bubbles: true }));
});
await new Promise((resolve) => setTimeout(resolve, 60));
assert.equal(input.value, 'abc');
assert.htmlEqual(target.innerHTML, `<p>ab</p><input />`);
await new Promise((resolve) => setTimeout(resolve, 60));
assert.equal(input.value, 'abc');
assert.htmlEqual(target.innerHTML, `<p>abc</p><input />`);
}
});

@ -0,0 +1,19 @@
<script>
let value = $state('a');
function push(value) {
// Cannot use a queue and flush it manually here, because we need the input to be focused
const deferred = Promise.withResolvers();
setTimeout(() => deferred.resolve(value), 100);
return deferred.promise;
}
</script>
<svelte:boundary>
<p>{await push(value)}</p>
<input bind:value />
{#snippet pending()}
<p>loading</p>
{/snippet}
</svelte:boundary>

@ -0,0 +1,27 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target }) {
const [input] = target.querySelectorAll('input');
assert.equal(input.value, '2');
assert.htmlEqual(target.innerHTML, `<p>2</p><input type="number" />`);
flushSync(() => {
input.focus();
input.value = '3';
input.dispatchEvent(new InputEvent('input', { bubbles: true }));
});
assert.equal(input.value, '3');
assert.htmlEqual(target.innerHTML, `<p>3</p><input type="number" />`);
flushSync(() => {
input.focus();
input.value = '6';
input.dispatchEvent(new InputEvent('input', { bubbles: true }));
});
assert.equal(input.value, '5');
assert.htmlEqual(target.innerHTML, `<p>5</p><input type="number" />`);
}
});

@ -0,0 +1,22 @@
<script>
let value = $state(0)
const min = 2
const max = 5
$effect(() => {
setValue()
})
function setValue() {
if (value < min) {
value = min
}
if (value > max) {
value = max
}
}
</script>
<p>{value}</p>
<input type="number" bind:value />
Loading…
Cancel
Save