Skip to content

Commit bf7de3a

Browse files
committed
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.
1 parent ce4a99e commit bf7de3a

File tree

7 files changed

+122
-14
lines changed

7 files changed

+122
-14
lines changed

.changeset/grumpy-boats-beg.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: keep input in sync when binding updated via effect

packages/svelte/src/internal/client/dom/elements/bindings/input.js

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
/** @import { Batch } from '../../../reactivity/batch.js' */
21
import { DEV } from 'esm-env';
32
import { render_effect, teardown } from '../../../reactivity/effects.js';
43
import { listen_to_event_and_reset_event } from './shared.js';
@@ -19,8 +18,6 @@ import { current_batch } from '../../../reactivity/batch.js';
1918
export function bind_value(input, get, set = get) {
2019
var runes = is_runes();
2120

22-
var batches = new WeakSet();
23-
2421
listen_to_event_and_reset_event(input, 'input', (is_reset) => {
2522
if (DEV && input.type === 'checkbox') {
2623
// TODO should this happen in prod too?
@@ -32,10 +29,6 @@ export function bind_value(input, get, set = get) {
3229
value = is_numberlike_input(input) ? to_number(value) : value;
3330
set(value);
3431

35-
if (current_batch !== null) {
36-
batches.add(current_batch);
37-
}
38-
3932
// In runes mode, respect any validation in accessors (doesn't apply in legacy mode,
4033
// because we use mutable state which ensures the render effect always runs)
4134
if (runes && value !== (value = get())) {
@@ -62,10 +55,6 @@ export function bind_value(input, get, set = get) {
6255
(untrack(get) == null && input.value)
6356
) {
6457
set(is_numberlike_input(input) ? to_number(input.value) : input.value);
65-
66-
if (current_batch !== null) {
67-
batches.add(current_batch);
68-
}
6958
}
7059

7160
render_effect(() => {
@@ -76,9 +65,9 @@ export function bind_value(input, get, set = get) {
7665

7766
var value = get();
7867

79-
if (input === document.activeElement && batches.has(/** @type {Batch} */ (current_batch))) {
80-
// Never rewrite the contents of a focused input. We can get here if, for example,
81-
// an update is deferred because of async work depending on the input:
68+
if (input === document.activeElement && current_batch?.flushing_async) {
69+
// Never rewrite the contents of a focused input when flushing async work.
70+
// We can get here if, for example, an update is deferred because of async work depending on the input:
8271
//
8372
// <input bind:value={query}>
8473
// <p>{await find(query)}</p>

packages/svelte/src/internal/client/reactivity/batch.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,11 @@ export class Batch {
152152
*/
153153
skipped_effects = new Set();
154154

155+
/**
156+
* True while a batch that had asynchronous work (i.e. a pending count) is being flushed.
157+
*/
158+
flushing_async = false;
159+
155160
/**
156161
*
157162
* @param {Effect[]} root_effects
@@ -412,6 +417,8 @@ export class Batch {
412417
this.#pending -= 1;
413418

414419
if (this.#pending === 0) {
420+
this.flushing_async = true;
421+
415422
for (const e of this.#render_effects) {
416423
set_signal_status(e, DIRTY);
417424
schedule_effect(e);
@@ -431,6 +438,8 @@ export class Batch {
431438
this.#effects = [];
432439

433440
this.flush();
441+
442+
this.flushing_async = true;
434443
} else {
435444
this.deactivate();
436445
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
async test({ assert, target }) {
6+
await new Promise((resolve) => setTimeout(resolve, 110));
7+
8+
const [input] = target.querySelectorAll('input');
9+
10+
assert.equal(input.value, 'a');
11+
assert.htmlEqual(target.innerHTML, `<p>a</p><input />`);
12+
13+
flushSync(() => {
14+
input.focus();
15+
input.value = 'ab';
16+
input.dispatchEvent(new InputEvent('input', { bubbles: true }));
17+
});
18+
19+
await new Promise((resolve) => setTimeout(resolve, 50));
20+
21+
flushSync(() => {
22+
input.focus();
23+
input.value = 'abc';
24+
input.dispatchEvent(new InputEvent('input', { bubbles: true }));
25+
});
26+
27+
await new Promise((resolve) => setTimeout(resolve, 60));
28+
29+
assert.equal(input.value, 'abc');
30+
assert.htmlEqual(target.innerHTML, `<p>ab</p><input />`);
31+
32+
await new Promise((resolve) => setTimeout(resolve, 60));
33+
34+
assert.equal(input.value, 'abc');
35+
assert.htmlEqual(target.innerHTML, `<p>abc</p><input />`);
36+
}
37+
});
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<script>
2+
let value = $state('a');
3+
4+
function push(value) {
5+
// Cannot use a queue and flush it manually here, because we need the input to be focused
6+
const deferred = Promise.withResolvers();
7+
setTimeout(() => deferred.resolve(value), 100);
8+
return deferred.promise;
9+
}
10+
</script>
11+
12+
<svelte:boundary>
13+
<p>{await push(value)}</p>
14+
<input bind:value />
15+
16+
{#snippet pending()}
17+
<p>loading</p>
18+
{/snippet}
19+
</svelte:boundary>
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
async test({ assert, target }) {
6+
const [input] = target.querySelectorAll('input');
7+
8+
assert.equal(input.value, '2');
9+
assert.htmlEqual(target.innerHTML, `<p>2</p><input type="number" />`);
10+
11+
flushSync(() => {
12+
input.focus();
13+
input.value = '3';
14+
input.dispatchEvent(new InputEvent('input', { bubbles: true }));
15+
});
16+
assert.equal(input.value, '3');
17+
assert.htmlEqual(target.innerHTML, `<p>3</p><input type="number" />`);
18+
19+
flushSync(() => {
20+
input.focus();
21+
input.value = '6';
22+
input.dispatchEvent(new InputEvent('input', { bubbles: true }));
23+
});
24+
assert.equal(input.value, '5');
25+
assert.htmlEqual(target.innerHTML, `<p>5</p><input type="number" />`);
26+
}
27+
});
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<script>
2+
let value = $state(0)
3+
4+
const min = 2
5+
const max = 5
6+
7+
$effect(() => {
8+
setValue()
9+
})
10+
11+
function setValue() {
12+
if (value < min) {
13+
value = min
14+
}
15+
if (value > max) {
16+
value = max
17+
}
18+
}
19+
</script>
20+
21+
<p>{value}</p>
22+
<input type="number" bind:value />

0 commit comments

Comments
 (0)