Merge remote-tracking branch 'origin/main' into 1000-reading-derived-effects

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

@@ -194,6 +194,8 @@ export class Batch {
 		// if we didn't start any new async work, and no async work
 		// is outstanding from a previous flush, commit
 		if (this.#async_effects.length === 0 && this.#pending === 0) {
+			this.#commit();
+
 			var render_effects = this.#render_effects;
 			var effects = this.#effects;
 
@@ -201,8 +203,6 @@ export class Batch {
 			this.#effects = [];
 			this.#block_effects = [];
 
-			this.#commit();
-
 			flush_queued_effects(render_effects);
 			flush_queued_effects(effects);
 
@@ -540,49 +540,49 @@ function flush_queued_effects(effects) {
 	var length = effects.length;
 	if (length === 0) return;
 
-	for (var i = 0; i < length; i++) {
-		var effect = effects[i];
-
-		if ((effect.f & (DESTROYED | INERT)) === 0) {
-			if (is_dirty(effect)) {
-				var wv = write_version;
-
-				// updating a derived for also increase the write version but that doesn't mean
-				// state was written to in the user effect...so we reset the derived writes
-				// before running the effect so that we can subtract the amount of derived writes
-				// from the write version when we detect if state was written to in the user effect
-				reset_derived_writes();
-
-				update_effect(effect);
-
-				// Effects with no dependencies or teardown do not get added to the effect tree.
-				// Deferred effects (e.g. `$effect(...)`) _are_ added to the tree because we
-				// don't know if we need to keep them until they are executed. Doing the check
-				// here (rather than in `update_effect`) allows us to skip the work for
-				// immediate effects.
-				if (effect.deps === null && effect.first === null && effect.nodes_start === null) {
-					// if there's no teardown or abort controller we completely unlink
-					// the effect from the graph
-					if (effect.teardown === null && effect.ac === null) {
-						// remove this effect from the graph
-						unlink_effect(effect);
-					} else {
-						// keep the effect in the graph, but free up some memory
-						effect.fn = null;
-					}
-				}
+	var i = 0;
+
+	while (i < length) {
+		var effect = effects[i++];
 
-				// 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 (write_version - get_derived_writes() > wv && (effect.f & USER_EFFECT) !== 0) {
-					break;
+		if ((effect.f & (DESTROYED | INERT)) === 0 && is_dirty(effect)) {
+			var wv = write_version;
+
+			// updating a derived for also increase the write version but that doesn't mean
+			// state was written to in the user effect...so we reset the derived writes
+			// before running the effect so that we can subtract the amount of derived writes
+			// from the write version when we detect if state was written to in the user effect
+			reset_derived_writes();
+
+			update_effect(effect);
+
+			// Effects with no dependencies or teardown do not get added to the effect tree.
+			// Deferred effects (e.g. `$effect(...)`) _are_ added to the tree because we
+			// don't know if we need to keep them until they are executed. Doing the check
+			// here (rather than in `update_effect`) allows us to skip the work for
+			// immediate effects.
+			if (effect.deps === null && effect.first === null && effect.nodes_start === null) {
+				// if there's no teardown or abort controller we completely unlink
+				// the effect from the graph
+				if (effect.teardown === null && effect.ac === null) {
+					// remove this effect from the graph
+					unlink_effect(effect);
+				} else {
+					// keep the effect in the graph, but free up some memory
+					effect.fn = null;
 				}
 			}
+
+			// 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 (write_version - get_derived_writes() > wv && (effect.f & USER_EFFECT) !== 0) {
+				break;
+			}
 		}
 	}
 
-	for (; i < length; i += 1) {
-		schedule_effect(effects[i]);
+	while (i < length) {
+		schedule_effect(effects[i++]);
 	}
 }
 

packages/svelte/tests/runtime-runes/samples/effect-order-7/_config.js

@@ -2,14 +2,18 @@ import { flushSync } from 'svelte';
 import { test } from '../../test';
 
 export default test({
-	skip: true,
+	// For this to work in non-async mode, we would need to abort
+	// inside `#traverse_effect_tree`, which would be very
+	// complicated and annoying. Since this hasn't been
+	// a real issue (AFAICT), we ignore it
+	skip_no_async: true,
 
-	async test({ assert, target, logs }) {
+	async test({ target }) {
 		const [open, close] = target.querySelectorAll('button');
 
 		flushSync(() => open.click());
-		flushSync(() => close.click());
 
-		assert.deepEqual(logs, [true]);
+		// if the effect queue isn't aborted after the state change, this will throw
+		flushSync(() => close.click());
 	}
 });