From 5e28508377058b19dd4628530eee788db1523679 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 26 Feb 2026 14:29:57 -0500 Subject: [PATCH] chore: simplify create effect (#17813) follow-up to #17808. This makes it a bit more explicit _why_ we do certain things in `create_effect`, and gets rid of a redundant parameter ### Before submitting the PR, please make sure you do the following - [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`. - [x] This message body should clearly illustrate what problems it solves. - [ ] Ideally, include a test that fails without this PR but passes with it. - [ ] If this PR changes code within `packages/svelte/src`, add a changeset (`npx changeset`). ### Tests and linting - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` --- .../src/internal/client/reactivity/effects.js | 80 ++++++++++--------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index e9cc72766a..b670e7ab55 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -80,10 +80,9 @@ function push_effect(effect, parent_effect) { /** * @param {number} type * @param {null | (() => void | (() => void))} fn - * @param {boolean} sync * @returns {Effect} */ -function create_effect(type, fn, sync) { +function create_effect(type, fn) { var parent = active_effect; if (DEV) { @@ -119,36 +118,39 @@ function create_effect(type, fn, sync) { effect.component_function = dev_current_component_function; } - if (sync) { + /** @type {Effect | null} */ + var e = effect; + + if ((type & EFFECT) !== 0) { + if (collected_effects !== null) { + // created during traversal — collect and run afterwards + collected_effects.push(effect); + } else { + // schedule for later + schedule_effect(effect); + } + } else if (fn !== null) { try { update_effect(effect); } catch (e) { destroy_effect(effect); throw e; } - } else if ((type & EFFECT) !== 0 && collected_effects !== null) { - collected_effects.push(effect); - } else if (fn !== null) { - schedule_effect(effect); - } - /** @type {Effect | null} */ - var e = effect; - - // if an effect has already ran and doesn't need to be kept in the tree - // (because it won't re-run, has no DOM, and has no teardown etc) - // then we skip it and go to its child (if any) - if ( - sync && - e.deps === null && - e.teardown === null && - e.nodes === null && - e.first === e.last && // either `null`, or a singular child - (e.f & EFFECT_PRESERVED) === 0 - ) { - e = e.first; - if ((type & BLOCK_EFFECT) !== 0 && (type & EFFECT_TRANSPARENT) !== 0 && e !== null) { - e.f |= EFFECT_TRANSPARENT; + // if an effect doesn't need to be kept in the tree (because it + // won't re-run, has no DOM, and has no teardown etc) + // then we skip it and go to its child (if any) + if ( + e.deps === null && + e.teardown === null && + e.nodes === null && + e.first === e.last && // either `null`, or a singular child + (e.f & EFFECT_PRESERVED) === 0 + ) { + e = e.first; + if ((type & BLOCK_EFFECT) !== 0 && (type & EFFECT_TRANSPARENT) !== 0 && e !== null) { + e.f |= EFFECT_TRANSPARENT; + } } } @@ -185,7 +187,7 @@ export function effect_tracking() { * @param {() => void} fn */ export function teardown(fn) { - const effect = create_effect(RENDER_EFFECT, null, false); + const effect = create_effect(RENDER_EFFECT, null); set_signal_status(effect, CLEAN); effect.teardown = fn; return effect; @@ -223,7 +225,7 @@ export function user_effect(fn) { * @param {() => void | (() => void)} fn */ export function create_user_effect(fn) { - return create_effect(EFFECT | USER_EFFECT, fn, false); + return create_effect(EFFECT | USER_EFFECT, fn); } /** @@ -238,12 +240,12 @@ export function user_pre_effect(fn) { value: '$effect.pre' }); } - return create_effect(RENDER_EFFECT | USER_EFFECT, fn, true); + return create_effect(RENDER_EFFECT | USER_EFFECT, fn); } /** @param {() => void | (() => void)} fn */ export function eager_effect(fn) { - return create_effect(EAGER_EFFECT, fn, true); + return create_effect(EAGER_EFFECT, fn); } /** @@ -253,7 +255,7 @@ export function eager_effect(fn) { */ export function effect_root(fn) { Batch.ensure(); - const effect = create_effect(ROOT_EFFECT | EFFECT_PRESERVED, fn, true); + const effect = create_effect(ROOT_EFFECT | EFFECT_PRESERVED, fn); return () => { destroy_effect(effect); @@ -267,7 +269,7 @@ export function effect_root(fn) { */ export function component_root(fn) { Batch.ensure(); - const effect = create_effect(ROOT_EFFECT | EFFECT_PRESERVED, fn, true); + const effect = create_effect(ROOT_EFFECT | EFFECT_PRESERVED, fn); return (options = {}) => { return new Promise((fulfil) => { @@ -289,7 +291,7 @@ export function component_root(fn) { * @returns {Effect} */ export function effect(fn) { - return create_effect(EFFECT, fn, false); + return create_effect(EFFECT, fn); } /** @@ -347,7 +349,7 @@ export function legacy_pre_effect_reset() { * @returns {Effect} */ export function async_effect(fn) { - return create_effect(ASYNC | EFFECT_PRESERVED, fn, true); + return create_effect(ASYNC | EFFECT_PRESERVED, fn); } /** @@ -355,7 +357,7 @@ export function async_effect(fn) { * @returns {Effect} */ export function render_effect(fn, flags = 0) { - return create_effect(RENDER_EFFECT | flags, fn, true); + return create_effect(RENDER_EFFECT | flags, fn); } /** @@ -366,7 +368,7 @@ export function render_effect(fn, flags = 0) { */ export function template_effect(fn, sync = [], async = [], blockers = []) { flatten(blockers, sync, async, (values) => { - create_effect(RENDER_EFFECT, () => fn(...values.map(get)), true); + create_effect(RENDER_EFFECT, () => fn(...values.map(get))); }); } @@ -383,7 +385,7 @@ export function deferred_template_effect(fn, sync = [], async = [], blockers = [ } flatten(blockers, sync, async, (values) => { - create_effect(EFFECT, () => fn(...values.map(get)), false); + create_effect(EFFECT, () => fn(...values.map(get))); if (decrement_pending) { decrement_pending(); @@ -396,7 +398,7 @@ export function deferred_template_effect(fn, sync = [], async = [], blockers = [ * @param {number} flags */ export function block(fn, flags = 0) { - var effect = create_effect(BLOCK_EFFECT | flags, fn, true); + var effect = create_effect(BLOCK_EFFECT | flags, fn); if (DEV) { effect.dev_stack = dev_stack; } @@ -408,7 +410,7 @@ export function block(fn, flags = 0) { * @param {number} flags */ export function managed(fn, flags = 0) { - var effect = create_effect(MANAGED_EFFECT | flags, fn, true); + var effect = create_effect(MANAGED_EFFECT | flags, fn); if (DEV) { effect.dev_stack = dev_stack; } @@ -419,7 +421,7 @@ export function managed(fn, flags = 0) { * @param {(() => void)} fn */ export function branch(fn) { - return create_effect(BRANCH_EFFECT | EFFECT_PRESERVED, fn, true); + return create_effect(BRANCH_EFFECT | EFFECT_PRESERVED, fn); } /**