Skip to content

Commit 9eb969d

Browse files
trueadmRich-Harris
andauthored
fix: address bug in before/after update (#9448)
* fix: address bug in before/after update fix: address bug in before/after update * Add changeset * use every instead of filter - more explicit and enables early-exit from the loop * Update logic and comment --------- Co-authored-by: Rich Harris <[email protected]>
1 parent f5101c0 commit 9eb969d

File tree

5 files changed

+60
-3
lines changed

5 files changed

+60
-3
lines changed

.changeset/nasty-clocks-exercise.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: corrects a beforeUpdate/afterUpdate bug

packages/svelte/src/internal/client/runtime.js

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -960,10 +960,18 @@ export function set_signal_value(signal, value) {
960960
schedule_effect(current_effect, false);
961961
}
962962
mark_signal_consumers(signal, DIRTY, true);
963-
// If we have afterUpdates locally on the component, but we're within a render effect
964-
// then we will need to manually invoke the beforeUpdate/afterUpdate logic.
963+
// This logic checks if there are any render effects queued after the above marking
964+
// of consumers. If there are render effects that have the same component context as
965+
// the source signal we're writing to, then we can bail-out of this logic as there
966+
// will be a render effect in the queue that hopefully takes case of triggering the
967+
// beforeUpdate/afterUpdate logic (doing it again here would duplicate them). However,
968+
// if the render effects scheduled in the queue are unrelated to the component context,
969+
// then we need to trigger the beforeUpdate/afterUpdate logic here instead.
965970
// TODO: should we put this being a is_runes check and only run it in non-runes mode?
966-
if (current_effect === null && current_queued_pre_and_render_effects.length === 0) {
971+
if (
972+
current_effect === null &&
973+
current_queued_pre_and_render_effects.every((e) => e.context !== component_context)
974+
) {
967975
const update_callbacks = component_context?.update_callbacks;
968976
if (update_callbacks != null) {
969977
update_callbacks.before.forEach(/** @param {any} c */ (c) => c());
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<script>
2+
const {count, increment} = $props();
3+
</script>
4+
5+
<button onclick={increment}>
6+
{count}
7+
</button>
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
html: '<button>0</button>',
6+
7+
async test({ assert, target, component }) {
8+
const [btn] = target.querySelectorAll('button');
9+
flushSync(() => {
10+
btn.click();
11+
});
12+
assert.deepEqual(component.log, ['beforeUpdate', 'afterUpdate']);
13+
assert.htmlEqual(target.innerHTML, `<button>1</button>`);
14+
}
15+
});
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<script>
2+
import Child from './Child.svelte'
3+
import {afterUpdate, beforeUpdate} from 'svelte';
4+
5+
const {log = []} = $props();
6+
7+
let count = $state(0);
8+
9+
const increment = () => {
10+
count++;
11+
}
12+
13+
beforeUpdate(() => {
14+
log.push('beforeUpdate');
15+
});
16+
17+
afterUpdate(() => {
18+
log.push('afterUpdate');
19+
});
20+
</script>
21+
22+
<Child count={count} increment={increment} />

0 commit comments

Comments
 (0)