fix: adjust render effect ordering (#10783)

We can simplify pre effects by not doing the flush logic at all now. Instead we can move the flushing logic to the only place its needed – for beforeUpdate

Author: trueadm

.changeset/tasty-steaks-smile.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: adjust render effect ordering

packages/svelte/src/internal/client/dom/legacy/lifecycle.js

@@ -1,6 +1,12 @@
 import { run } from '../../../common.js';
 import { pre_effect, user_effect } from '../../reactivity/effects.js';
-import { current_component_context, deep_read_state, get, untrack } from '../../runtime.js';
+import {
+	current_component_context,
+	deep_read_state,
+	flush_local_render_effects,
+	get,
+	untrack
+} from '../../runtime.js';
 
 /**
  * Legacy-mode only: Call `onMount` callbacks and set up `beforeUpdate`/`afterUpdate` effects
@@ -16,6 +22,10 @@ export function init() {
 		pre_effect(() => {
 			observe_all(context);
 			callbacks.b.forEach(run);
+			// beforeUpdate might change state that affects rendering, ensure the render effects following from it
+			// are batched up with the current run. Avoids for example child components rerunning when they're
+			// now hidden because beforeUpdate did set an if block to false.
+			flush_local_render_effects();
 		});
 	}
 

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

@@ -148,18 +148,9 @@ export function pre_effect(fn) {
 					: '')
 		);
 	}
-
 	const sync = current_effect !== null && (current_effect.f & RENDER_EFFECT) !== 0;
 
-	return create_effect(
-		PRE_EFFECT,
-		() => {
-			const val = fn();
-			flush_local_render_effects();
-			return val;
-		},
-		sync
-	);
+	return create_effect(PRE_EFFECT, fn, sync);
 }
 
 /**

packages/svelte/src/reactivity/set.js

@@ -82,9 +82,11 @@ export class ReactiveSet extends Set {
 	/** @param {T} value */
 	has(value) {
 		var source = this.#sources.get(value);
+		// We should always track the version in case
+		// the Set ever gets this value in the future.
+		get(this.#version);
 
 		if (source === undefined) {
-			get(this.#version);
 			return false;
 		}
 

packages/svelte/src/reactivity/set.test.ts

@@ -30,9 +30,7 @@ test('set.values()', () => {
 		set.clear();
 	});
 
-	// TODO looks like another effect ordering bug — sequence should be <size, has, values>,
-	// but values is reversed at end
-	assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, [], false]);
+	assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]);
 
 	cleanup();
 });

packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-5/_config.js

@@ -0,0 +1,21 @@
+import { test } from '../../test';
+import { log } from './log.js';
+
+export default test({
+	get props() {
+		return { n: 0 };
+	},
+
+	before_test() {
+		log.length = 0;
+	},
+
+	async test({ assert, component }) {
+		assert.deepEqual(log, ['$effect.pre 0', 'another $effect.pre 1', 'render n0', 'render i1']);
+
+		log.length = 0;
+		component.n += 1;
+
+		assert.deepEqual(log, ['$effect.pre 1', 'another $effect.pre 2', 'render n1', 'render i2']);
+	}
+});

packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-5/log.js

@@ -0,0 +1,2 @@
+/** @type {any[]} */
+export const log = [];

packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-5/main.svelte

@@ -0,0 +1,19 @@
+<script>
+	import { untrack } from 'svelte';
+	import { log } from './log.js';
+	let { n = 0 } = $props();
+	let i = $state(0);
+	function logRender(i) {
+		log.push(`render ${i}`);
+	}
+	$effect.pre(() => {
+		log.push(`$effect.pre ${n}`);
+		untrack(() => i++)
+	});
+	$effect.pre(() => {
+		log.push('another $effect.pre '+ i);
+	})
+</script>
+
+<p>{logRender(`n${n}`)}</p>
+<p>{logRender(`i${i}`)}</p>