Revert "chore: apply each block controlled teardown optimization (#11045)" (#11049)

This reverts commit 1afec80.

Author: Rich-Harris

packages/svelte/src/internal/client/dom/blocks/each.js

@@ -15,11 +15,9 @@ import { untrack } from '../../runtime.js';
 import {
 	block,
 	branch,
-	destroy_effect,
 	effect,
-	run_out_transitions,
-	pause_children,
 	pause_effect,
+	pause_effects,
 	resume_effect
 } from '../../reactivity/effects.js';
 import { source, mutable_source, set } from '../../reactivity/sources.js';
@@ -41,39 +39,6 @@ export function set_current_each_item(item) {
 	current_each_item = item;
 }
 
-/**
- * Pause multiple effects simultaneously, and coordinate their
- * subsequent destruction. Used in each blocks
- * @param {import('#client').Effect[]} effects
- * @param {null | Node} controlled_anchor
- * @param {() => void} [callback]
- */
-function pause_effects(effects, controlled_anchor, callback) {
-	/** @type {import('#client').TransitionManager[]} */
-	var transitions = [];
-	var length = effects.length;
-
-	for (var i = 0; i < length; i++) {
-		pause_children(effects[i], transitions, true);
-	}
-
-	// If we have a controlled anchor, it means that the each block is inside a single
-	// DOM element, so we can apply a fast-path for clearing the contents of the element.
-	if (effects.length > 0 && transitions.length === 0 && controlled_anchor !== null) {
-		var parent_node = /** @type {Element} */ (controlled_anchor.parentNode);
-		parent_node.textContent = '';
-		parent_node.append(controlled_anchor);
-	}
-
-	run_out_transitions(transitions, () => {
-		for (var i = 0; i < length; i++) {
-			destroy_effect(effects[i]);
-		}
-
-		if (callback !== undefined) callback();
-	});
-}
-
 /**
  * @template V
  * @param {Element | Comment} anchor The next sibling node, or the parent node if this is a 'controlled' block
@@ -180,6 +145,7 @@ function each(anchor, flags, get_collection, get_key, render_fn, fallback_fn, re
 		}
 
 		if (!hydrating) {
+			// TODO add 'empty controlled block' optimisation here
 			reconcile_fn(array, state, anchor, render_fn, flags, keys);
 		}
 
@@ -278,9 +244,7 @@ function reconcile_indexed_array(array, state, anchor, render_fn, flags) {
 			effects.push(a_items[i].e);
 		}
 
-		var controlled_anchor = (flags & EACH_IS_CONTROLLED) !== 0 && b === 0 ? anchor : null;
-
-		pause_effects(effects, controlled_anchor, () => {
+		pause_effects(effects, () => {
 			state.items.length = b;
 		});
 	}
@@ -310,7 +274,6 @@ function reconcile_tracked_array(array, state, anchor, render_fn, flags, keys) {
 
 	var is_animated = (flags & EACH_IS_ANIMATED) !== 0;
 	var should_update = (flags & (EACH_ITEM_REACTIVE | EACH_INDEX_REACTIVE)) !== 0;
-	var is_controlled = (flags & EACH_IS_CONTROLLED) !== 0;
 	var start = 0;
 	var item;
 
@@ -418,11 +381,6 @@ function reconcile_tracked_array(array, state, anchor, render_fn, flags, keys) {
 		// I fully understand this part)
 		if (moved) {
 			mark_lis(sources);
-		} else if (is_controlled && to_destroy.length === a_items.length) {
-			// We can optimize the case in which all items are replaced —
-			// destroy everything first, then append new items
-			pause_effects(to_destroy, anchor);
-			to_destroy = [];
 		}
 
 		// working from the back, insert new or moved items
@@ -463,9 +421,9 @@ function reconcile_tracked_array(array, state, anchor, render_fn, flags, keys) {
 		});
 	}
 
-	var controlled_anchor = is_controlled && b === 0 ? anchor : null;
-
-	pause_effects(to_destroy, controlled_anchor, () => {
+	// TODO: would be good to avoid this closure in the case where we have no
+	// transitions at all. It would make it far more JIT friendly in the hot cases.
+	pause_effects(to_destroy, () => {
 		state.items = b_items;
 	});
 }

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

@@ -27,6 +27,7 @@ import {
 	IS_ELSEIF
 } from '../constants.js';
 import { set } from './sources.js';
+import { noop } from '../../shared/utils.js';
 import { remove } from '../dom/reconciler.js';
 
 /**
@@ -294,25 +295,50 @@ export function destroy_effect(effect) {
  * completed, and if the state change is reversed then we _resume_ it.
  * A paused effect does not update, and the DOM subtree becomes inert.
  * @param {import('#client').Effect} effect
- * @param {() => void} [callback]
+ * @param {() => void} callback
  */
-export function pause_effect(effect, callback) {
+export function pause_effect(effect, callback = noop) {
 	/** @type {import('#client').TransitionManager[]} */
 	var transitions = [];
 
 	pause_children(effect, transitions, true);
 
-	run_out_transitions(transitions, () => {
+	out(transitions, () => {
 		destroy_effect(effect);
-		if (callback) callback();
+		callback();
+	});
+}
+
+/**
+ * Pause multiple effects simultaneously, and coordinate their
+ * subsequent destruction. Used in each blocks
+ * @param {import('#client').Effect[]} effects
+ * @param {() => void} callback
+ */
+export function pause_effects(effects, callback = noop) {
+	/** @type {import('#client').TransitionManager[]} */
+	var transitions = [];
+	var length = effects.length;
+
+	for (var i = 0; i < length; i++) {
+		pause_children(effects[i], transitions, true);
+	}
+
+	// TODO: would be good to avoid this closure in the case where we have no
+	// transitions at all. It would make it far more JIT friendly in the hot cases.
+	out(transitions, () => {
+		for (var i = 0; i < length; i++) {
+			destroy_effect(effects[i]);
+		}
+		callback();
 	});
 }
 
 /**
  * @param {import('#client').TransitionManager[]} transitions
  * @param {() => void} fn
  */
-export function run_out_transitions(transitions, fn) {
+function out(transitions, fn) {
 	var remaining = transitions.length;
 	if (remaining > 0) {
 		var check = () => --remaining || fn();
@@ -329,7 +355,7 @@ export function run_out_transitions(transitions, fn) {
  * @param {import('#client').TransitionManager[]} transitions
  * @param {boolean} local
  */
-export function pause_children(effect, transitions, local) {
+function pause_children(effect, transitions, local) {
 	if ((effect.f & INERT) !== 0) return;
 	effect.f ^= INERT;