Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: address bug in before/after update #9448

Merged
merged 4 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/nasty-clocks-exercise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: corrects a beforeUpdate/afterUpdate bug
14 changes: 11 additions & 3 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -960,10 +960,18 @@ export function set_signal_value(signal, value) {
schedule_effect(current_effect, false);
}
mark_signal_consumers(signal, DIRTY, true);
// If we have afterUpdates locally on the component, but we're within a render effect
// then we will need to manually invoke the beforeUpdate/afterUpdate logic.
// This logic checks if there are any render effects queued after the above marking
// of consumers. If there are render effects that have the same component context as
// the source signal we're writing to, then we can bail-out of this logic as there
// will be a render effect in the queue that hopefully takes case of triggering the
// beforeUpdate/afterUpdate logic (doing it again here would duplicate them). However,
// if the render effects scheduled in the queue are unrelated to the component context,
// then we need to trigger the beforeUpdate/afterUpdate logic here instead.
// TODO: should we put this being a is_runes check and only run it in non-runes mode?
if (current_effect === null && current_queued_pre_and_render_effects.length === 0) {
if (
current_effect === null &&
current_queued_pre_and_render_effects.every((e) => e.context !== component_context)
trueadm marked this conversation as resolved.
Show resolved Hide resolved
) {
const update_callbacks = component_context?.update_callbacks;
if (update_callbacks != null) {
update_callbacks.before.forEach(/** @param {any} c */ (c) => c());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
const {count, increment} = $props();
</script>

<button onclick={increment}>
{count}
</button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { flushSync } from 'svelte';
import { test } from '../../test';

export default test({
html: '<button>0</button>',

async test({ assert, target, component }) {
const [btn] = target.querySelectorAll('button');
flushSync(() => {
btn.click();
});
assert.deepEqual(component.log, ['beforeUpdate', 'afterUpdate']);
assert.htmlEqual(target.innerHTML, `<button>1</button>`);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<script>
import Child from './Child.svelte'
import {afterUpdate, beforeUpdate} from 'svelte';

const {log = []} = $props();

let count = $state(0);

const increment = () => {
count++;
}

beforeUpdate(() => {
log.push('beforeUpdate');
});

afterUpdate(() => {
log.push('afterUpdate');
});
</script>

<Child count={count} increment={increment} />