Skip to content

Commit

Permalink
Revert "chore: apply each block controlled teardown optimization (#11045
Browse files Browse the repository at this point in the history
)" (#11049)

This reverts commit 1afec80.
  • Loading branch information
Rich-Harris authored Apr 3, 2024
1 parent bb1d229 commit f7c8fd5
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 54 deletions.
54 changes: 6 additions & 48 deletions packages/svelte/src/internal/client/dom/blocks/each.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
});
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
});
}
Expand Down
38 changes: 32 additions & 6 deletions packages/svelte/src/internal/client/reactivity/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -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();
Expand All @@ -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;

Expand Down

0 comments on commit f7c8fd5

Please sign in to comment.