fix: ensure child effects are destroyed before their deriveds (#14043)

* fix: ensure legacy props cache last value when destroyed

* fix runes

* better approach

* better approach

* kill code

* lint

Author: trueadm

.changeset/khaki-carrots-mix.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure child effects are destroyed before their deriveds

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

@@ -437,8 +437,8 @@ export function destroy_effect(effect, remove_dom = true) {
 		removed = true;
 	}
 
-	destroy_effect_deriveds(effect);
 	destroy_effect_children(effect, remove_dom && !removed);
+	destroy_effect_deriveds(effect);
 	remove_reactions(effect, 0);
 	set_signal_status(effect, DESTROYED);
 

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

@@ -1,4 +1,4 @@
-/** @import { Derived, Source } from './types.js' */
+/** @import { Source } from './types.js' */
 import { DEV } from 'esm-env';
 import {
 	PROPS_IS_BINDABLE,
@@ -12,7 +12,6 @@ import { mutable_source, set, source } from './sources.js';
 import { derived, derived_safe_equal } from './deriveds.js';
 import {
 	active_effect,
-	active_reaction,
 	get,
 	is_signals_recorded,
 	set_active_effect,
@@ -21,7 +20,7 @@ import {
 } from '../runtime.js';
 import { safe_equals } from './equality.js';
 import * as e from '../errors.js';
-import { BRANCH_EFFECT, DESTROYED, LEGACY_DERIVED_PROP, ROOT_EFFECT } from '../constants.js';
+import { BRANCH_EFFECT, LEGACY_DERIVED_PROP, ROOT_EFFECT } from '../constants.js';
 import { proxy } from '../proxy.js';
 import { capture_store_binding } from './store.js';
 
@@ -369,17 +368,12 @@ export function prop(props, key, flags, fallback) {
 	// The derived returns the current value. The underlying mutable
 	// source is written to from various places to persist this value.
 	var inner_current_value = mutable_source(prop_value);
-
 	var current_value = with_parent_branch(() =>
 		derived(() => {
 			var parent_value = getter();
 			var child_value = get(inner_current_value);
-			var current_derived = /** @type {Derived} */ (active_reaction);
 
-			// If the getter from the parent returns undefined, switch
-			// to using the local value from inner_current_value instead,
-			// as the parent value might have been torn down
-			if (from_child || (parent_value === undefined && (current_derived.f & DESTROYED) !== 0)) {
+			if (from_child) {
 				from_child = false;
 				was_from_child = true;
 				return child_value;

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

@@ -432,12 +432,12 @@ export function update_effect(effect) {
 	}
 
 	try {
-		destroy_effect_deriveds(effect);
 		if ((flags & BLOCK_EFFECT) !== 0) {
 			destroy_block_effect_children(effect);
 		} else {
 			destroy_effect_children(effect);
 		}
+		destroy_effect_deriveds(effect);
 
 		execute_effect_teardown(effect);
 		var teardown = update_reaction(effect);

packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/Child.svelte

@@ -0,0 +1,11 @@
+<script>
+	import { onDestroy } from 'svelte';
+
+	export let data;
+	
+	onDestroy(() => {
+		data;
+	});
+</script>
+
+{data ? '' : null}

packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/_config.js

@@ -0,0 +1,10 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	test({ assert, logs, target }) {
+		target.querySelector('button')?.click();
+		flushSync();
+		assert.deepEqual(logs, ['should fire once']);
+	}
+});

packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/main.svelte

@@ -0,0 +1,17 @@
+<script>
+	import Child from './Child.svelte';
+
+	let active = true;
+	let data = { example: 'This is some example data' };
+
+	function log(data) {
+		console.log('should fire once');
+		return data;
+	}
+</script>
+
+<button on:click={() => active = false}>Hide</button>
+
+{#if active}
+	<Child data={log(data)} />
+{/if}

packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/Child.svelte

@@ -0,0 +1,11 @@
+<script>
+	import { onDestroy } from 'svelte';
+
+	const { data = 123 } = $props();
+
+	onDestroy(() => {
+		data;
+	});
+</script>
+
+{data ? '' : null}

packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/_config.js

@@ -0,0 +1,10 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	test({ assert, logs, target }) {
+		target.querySelector('button')?.click();
+		flushSync();
+		assert.deepEqual(logs, ['should fire once']);
+	}
+});

packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/main.svelte

@@ -0,0 +1,17 @@
+<script>
+	import Child from './Child.svelte';
+
+	let active = $state(true);
+	let data = $state({ example: 'This is some example data' });
+
+	function log(data) {
+		console.log('should fire once');
+		return data;
+	}
+</script>
+
+<button onclick={() => (active = false)}>Hide</button>
+
+{#if active}
+	<Child data={log(data)} />
+{/if}