Skip to content

Commit ece2e83

Browse files
Rich-Harrishmnd
andauthored
fix: increment signal versions when discarding forks (#17577)
* fix: always update an UNINITIALIZED derived on read Co-authored-by: David Roizenman <hmnd@users.noreply.github.com> * obsolete comments * rename test * add tests, fix * revert * Update .changeset/vast-hornets-draw.md --------- Co-authored-by: David Roizenman <hmnd@users.noreply.github.com>
1 parent 37cd40d commit ece2e83

File tree

8 files changed

+191
-0
lines changed

8 files changed

+191
-0
lines changed

.changeset/vast-hornets-draw.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: increment signal versions when discarding forks

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,13 @@ export function fork(fn) {
972972
await settled;
973973
},
974974
discard: () => {
975+
// cause any MAYBE_DIRTY deriveds to update
976+
// if they depend on things thath changed
977+
// inside the discarded fork
978+
for (var source of batch.current.keys()) {
979+
source.wv = increment_write_version();
980+
}
981+
975982
if (!committed && batches.has(batch)) {
976983
batches.delete(batch);
977984
batch.discard();
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
skip_no_async: true,
6+
async test({ assert, target }) {
7+
const [fork, toggle, increment] = target.querySelectorAll('button');
8+
9+
// derived is first evaluated in block effect, then discarded
10+
flushSync(() => fork.click());
11+
12+
// should not throw "Cannot convert a Symbol value to a string" due to cached UNINITIALIZED from first fork
13+
flushSync(() => fork.click());
14+
15+
// should not reflect the temporary change to `clicks` inside the fork
16+
flushSync(() => toggle.click());
17+
18+
assert.htmlEqual(
19+
target.innerHTML,
20+
`
21+
<button>fork</button>
22+
<button>toggle</button>
23+
<button>clicks: 0</button>
24+
<p>0</p>
25+
`
26+
);
27+
28+
flushSync(() => increment.click());
29+
30+
assert.htmlEqual(
31+
target.innerHTML,
32+
`
33+
<button>fork</button>
34+
<button>toggle</button>
35+
<button>clicks: 1</button>
36+
<p>2</p>
37+
`
38+
);
39+
}
40+
});
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<script>
2+
import { fork } from "svelte";
3+
4+
let clicks = $state(0);
5+
let show = $state(false);
6+
const derived = $derived(clicks * 2);
7+
</script>
8+
9+
<button onclick={() => {
10+
fork(() => {
11+
clicks += 1;
12+
show = true;
13+
}).discard();
14+
}}>fork</button>
15+
16+
<button onclick={() => show = !show}>toggle</button>
17+
18+
<button onclick={() => clicks++}>clicks: {clicks}</button>
19+
20+
{#if show}
21+
<p>{derived}</p>
22+
{/if}
23+
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
skip_no_async: true,
6+
async test({ assert, target }) {
7+
const [fork, toggle, increment] = target.querySelectorAll('button');
8+
9+
// derived is first evaluated in block effect, then discarded
10+
flushSync(() => fork.click());
11+
12+
// should not reflect the temporary change to `clicks` inside the fork
13+
// or throw "Cannot convert a Symbol value to a string" due to cached UNINITIALIZED
14+
flushSync(() => toggle.click());
15+
16+
assert.htmlEqual(
17+
target.innerHTML,
18+
`
19+
<button>fork</button>
20+
<button>toggle</button>
21+
<button>clicks: 0</button>
22+
<p>0</p>
23+
`
24+
);
25+
26+
flushSync(() => increment.click());
27+
28+
assert.htmlEqual(
29+
target.innerHTML,
30+
`
31+
<button>fork</button>
32+
<button>toggle</button>
33+
<button>clicks: 1</button>
34+
<p>2</p>
35+
`
36+
);
37+
}
38+
});
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<script>
2+
import { fork } from "svelte";
3+
4+
let clicks = $state(0);
5+
let show = $state(false);
6+
const derived = $derived(clicks * 2);
7+
</script>
8+
9+
<button onclick={() => {
10+
fork(() => {
11+
clicks += 1;
12+
show = true;
13+
}).discard();
14+
}}>fork</button>
15+
16+
<button onclick={() => show = !show}>toggle</button>
17+
18+
<button onclick={() => clicks++}>clicks: {clicks}</button>
19+
20+
{#if show}
21+
<p>{derived}</p>
22+
{/if}
23+
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
skip_no_async: true,
6+
async test({ assert, target }) {
7+
const [fork, toggle, increment] = target.querySelectorAll('button');
8+
9+
// initialize derived by showing it
10+
flushSync(() => toggle.click());
11+
flushSync(() => toggle.click());
12+
13+
// increment clicks
14+
flushSync(() => increment.click());
15+
16+
// update derived, but without writing to `derived.v`
17+
flushSync(() => fork.click());
18+
19+
// show derived
20+
flushSync(() => toggle.click());
21+
22+
assert.htmlEqual(
23+
target.innerHTML,
24+
`
25+
<button>fork</button>
26+
<button>toggle</button>
27+
<button>clicks: 1</button>
28+
<p>2</p>
29+
`
30+
);
31+
}
32+
});
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<script>
2+
import { fork } from "svelte";
3+
4+
let clicks = $state(0);
5+
let show = $state(false);
6+
const derived = $derived(clicks * 2);
7+
</script>
8+
9+
<button onclick={() => {
10+
fork(() => {
11+
clicks += 1;
12+
show = true;
13+
}).discard();
14+
}}>fork</button>
15+
16+
<button onclick={() => show = !show}>toggle</button>
17+
18+
<button onclick={() => clicks++}>clicks: {clicks}</button>
19+
20+
{#if show}
21+
<p>{derived}</p>
22+
{/if}
23+

0 commit comments

Comments
 (0)