Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
fix: skip effects inside dynamic component that is about to be destroyed
When a dynamic component was updated to a different instance and its props were updated at the same time, effects inside the component were still called with the already-changed props.
The fix is to mark the branch as skipped to never got to those effects.

Fixes #16387
  • Loading branch information
dummdidumm committed Aug 11, 2025
commit 7a153cefe49181006952519eb0634de833ad900e
5 changes: 5 additions & 0 deletions .changeset/neat-files-do.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: skip effects inside dynamic component that is about to be destroyed
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,10 @@ export function component(node, get_component, render_fn) {
if (defer) {
offscreen_fragment = document.createDocumentFragment();
offscreen_fragment.append((target = create_text()));
if (effect) {
/** @type {Batch} */ (current_batch).skipped_effects.add(effect);
}
}

pending_effect = branch(() => render_fn(target, component));
}

Expand Down
5 changes: 4 additions & 1 deletion packages/svelte/src/internal/client/reactivity/batch.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
RENDER_EFFECT,
ROOT_EFFECT,
USER_EFFECT,
MAYBE_DIRTY
MAYBE_DIRTY,
EFFECT_RAN
} from '#client/constants';
import { async_mode_flag } from '../../flags/index.js';
import { deferred, define_property } from '../../shared/utils.js';
Expand Down Expand Up @@ -599,6 +600,7 @@ function flush_queued_effects(effects) {

if ((effect.f & (DESTROYED | INERT)) === 0 && is_dirty(effect)) {
var n = current_batch ? current_batch.current.size : 0;
var ran = effect.f & EFFECT_RAN;

update_effect(effect);

Expand All @@ -622,6 +624,7 @@ function flush_queued_effects(effects) {
// if state is written in a user effect, abort and re-schedule, lest we run
// effects that should be removed as a result of the state change
if (
// ran &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be completely removed or uncommented?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh woops I didn't mean to commit that file

current_batch !== null &&
current_batch.current.size > n &&
(effect.f & USER_EFFECT) !== 0
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
let { data } = $props();
</script>

{#each data.obj.arr as i}
<p>{i}</p>
{/each}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>Comp 2</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { flushSync } from 'svelte';
import { test } from '../../test';

export default test({
async test({ assert, target }) {
const [btn] = target.querySelectorAll('button');
btn.click();
flushSync();
assert.htmlEqual(target.innerHTML, `<button>Change</button> <p>Comp 2</p>`);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<script>
import Comp_1 from './Comp-1.svelte';
import Comp_2 from './Comp-2.svelte';

let Comp = $state.raw(Comp_1);
let data = $state.raw({ obj: { arr: [1, 2, 3] } });

function change() {
Comp = Comp_2;
data = {};
}
</script>

<button onclick={change}>Change</button>

<Comp {data} />
Loading