Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: simplify effects further #11570

Merged
merged 1 commit into from
May 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 1 addition & 16 deletions packages/svelte/src/internal/client/dom/legacy/lifecycle.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
import { CLEAN } from '../../constants.js';
import { run, run_all } from '../../../shared/utils.js';
import { user_pre_effect, user_effect } from '../../reactivity/effects.js';
import {
current_component_context,
current_effect,
deep_read_state,
flush_local_render_effects,
get,
untrack
} from '../../runtime.js';
import { current_component_context, deep_read_state, get, untrack } from '../../runtime.js';

/**
* Legacy-mode only: Call `onMount` callbacks and set up `beforeUpdate`/`afterUpdate` effects
Expand All @@ -26,13 +18,6 @@ export function init() {
user_pre_effect(() => {
observe_all(context);
run_all(callbacks.b);
// 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.
const parent = current_effect?.parent;
if (parent != null && (parent.f & CLEAN) === 0) {
flush_local_render_effects(parent);
}
});
}

Expand Down
86 changes: 31 additions & 55 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -516,8 +516,27 @@ function infinite_loop_guard() {
*/
function flush_queued_root_effects(root_effects) {
infinite_loop_guard();
for (var i = 0; i < root_effects.length; i++) {
flush_nested_effects(root_effects[i]);

var previously_flushing_effect = is_flushing_effect;
is_flushing_effect = true;

try {
for (var i = 0; i < root_effects.length; i++) {
var effect = root_effects[i];

// When working with custom elements, the root effects might not have a root
if (effect.first === null && (effect.f & BRANCH_EFFECT) === 0) {
flush_queued_effects([effect]);
} else {
/** @type {import('#client').Effect[]} */
var collected_effects = [];

process_effects(effect, collected_effects);
flush_queued_effects(collected_effects);
}
}
} finally {
is_flushing_effect = previously_flushing_effect;
}
}

Expand Down Expand Up @@ -586,11 +605,10 @@ export function schedule_effect(signal) {
* effects to be flushed.
*
* @param {import('#client').Effect} effect
* @param {boolean} recursive
* @param {import('#client').Effect[]} collected_effects
* @returns {void}
*/
function process_effects(effect, recursive, collected_effects) {
function process_effects(effect, collected_effects) {
var current_effect = effect.first;
var effects = [];

Expand All @@ -614,13 +632,14 @@ function process_effects(effect, recursive, collected_effects) {
// Child might have been mutated since running the effect
child = current_effect.first;
}
if (recursive && child !== null) {

if (child !== null) {
current_effect = child;
continue;
}
} else if ((flags & EFFECT) !== 0) {
if (is_branch || is_clean) {
if (recursive && child !== null) {
if (child !== null) {
current_effect = child;
continue;
}
Expand Down Expand Up @@ -650,58 +669,15 @@ function process_effects(effect, recursive, collected_effects) {
current_effect = sibling;
}

if (recursive) {
// We might be dealing with many effects here, far more than can be spread into
// an array push call (callstack overflow). So let's deal with each effect in a loop.
for (var i = 0; i < effects.length; i++) {
child = effects[i];
collected_effects.push(child);
process_effects(child, true, collected_effects);
}
// We might be dealing with many effects here, far more than can be spread into
// an array push call (callstack overflow). So let's deal with each effect in a loop.
for (var i = 0; i < effects.length; i++) {
child = effects[i];
collected_effects.push(child);
process_effects(child, collected_effects);
}
}

/**
*
* This function recursively collects effects in topological order from the starting effect passed in.
* Effects will be collected when they match the filtered bitwise flag passed in only. The collected
* array will be populated with all the effects.
*
* @param {import('#client').Effect} effect
* @param {boolean} [recursive]
* @returns {void}
*/
function flush_nested_effects(effect, recursive = true) {
var previously_flushing_effect = is_flushing_effect;
is_flushing_effect = true;

try {
// When working with custom elements, the root effects might not have a root
if (effect.first === null && (effect.f & BRANCH_EFFECT) === 0) {
flush_queued_effects([effect]);
} else {
/** @type {import('#client').Effect[]} */
var collected_effects = [];

process_effects(effect, recursive, collected_effects);
flush_queued_effects(collected_effects);
}
} finally {
is_flushing_effect = previously_flushing_effect;
}
}

/**
* @param {import('#client').Effect} effect
* @returns {void}
*/
export function flush_local_render_effects(effect) {
infinite_loop_guard();
// We are entering a new flush sequence, so ensure counter is reset.
flush_count = 0;
flush_nested_effects(effect, false);
}

/**
* Internal version of `flushSync` with the option to not flush previous effects.
* Returns the result of the passed function, if given.
Expand Down
Loading