From 51736e576d86cc8879211632e3969730c513236b Mon Sep 17 00:00:00 2001 From: dabund24 Date: Mon, 20 Apr 2026 14:18:56 +0200 Subject: [PATCH 1/2] fix: do not dispatch transition event with animation (#18122) closes #18056 related: #17567 and #14009 # Changes move `dispatch_event()` calls in `transitions.js` out of `animate()` function using an additional `on_begin()` callback parameter. Doing so makes it possible to dispatch the `introstart` and `outrostart` events only from `transition()`. # Testing add a test checking that svelte dispatches no event when it runs an animation --- .changeset/hip-flowers-give.md | 5 ++ .../client/dom/elements/transitions.js | 68 +++++++++++++------ .../animate-no-transition-events/_config.js | 31 +++++++++ .../animate-no-transition-events/main.svelte | 19 ++++++ 4 files changed, 102 insertions(+), 21 deletions(-) create mode 100644 .changeset/hip-flowers-give.md create mode 100644 packages/svelte/tests/runtime-runes/samples/animate-no-transition-events/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/animate-no-transition-events/main.svelte diff --git a/.changeset/hip-flowers-give.md b/.changeset/hip-flowers-give.md new file mode 100644 index 0000000000..77f4dd8892 --- /dev/null +++ b/.changeset/hip-flowers-give.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: do not dispatch introstart event with animation of animate directive diff --git a/packages/svelte/src/internal/client/dom/elements/transitions.js b/packages/svelte/src/internal/client/dom/elements/transitions.js index 0c5431f030..7c4d385807 100644 --- a/packages/svelte/src/internal/client/dom/elements/transitions.js +++ b/packages/svelte/src/internal/client/dom/elements/transitions.js @@ -115,10 +115,17 @@ export function animation(element, get_fn, get_params) { ) { const options = get_fn()(this.element, { from, to }, get_params?.()); - animation = animate(this.element, options, undefined, 1, () => { - animation?.abort(); - animation = undefined; - }); + animation = animate( + this.element, + options, + undefined, + 1, + () => {}, + () => { + animation?.abort(); + animation = undefined; + } + ); } }, fix() { @@ -239,15 +246,24 @@ export function transition(flags, element, get_fn, get_params) { intro?.abort(); } - intro = animate(element, get_options(), outro, 1, () => { - dispatch_event(element, 'introend'); - - // Ensure we cancel the animation to prevent leaking - intro?.abort(); - intro = current_options = undefined; - - element.style.overflow = overflow; - }); + intro = animate( + element, + get_options(), + outro, + 1, + () => { + dispatch_event(element, 'introstart'); + }, + () => { + dispatch_event(element, 'introend'); + + // Ensure we cancel the animation to prevent leaking + intro?.abort(); + intro = current_options = undefined; + + element.style.overflow = overflow; + } + ); }, out(fn) { if (!is_outro) { @@ -258,10 +274,19 @@ export function transition(flags, element, get_fn, get_params) { element.inert = true; - outro = animate(element, get_options(), intro, 0, () => { - dispatch_event(element, 'outroend'); - fn?.(); - }); + outro = animate( + element, + get_options(), + intro, + 0, + () => { + dispatch_event(element, 'outrostart'); + }, + () => { + dispatch_event(element, 'outroend'); + fn?.(); + } + ); }, stop: () => { intro?.abort(); @@ -306,10 +331,11 @@ export function transition(flags, element, get_fn, get_params) { * @param {AnimationConfig | ((opts: { direction: 'in' | 'out' }) => AnimationConfig)} options * @param {Animation | undefined} counterpart The corresponding intro/outro to this outro/intro * @param {number} t2 The target `t` value — `1` for intro, `0` for outro + * @param {(() => void)} on_begin Called just before beginning the animation * @param {(() => void)} on_finish Called after successfully completing the animation * @returns {Animation} */ -function animate(element, options, counterpart, t2, on_finish) { +function animate(element, options, counterpart, t2, on_begin, on_finish) { var is_intro = t2 === 1; if (is_function(options)) { @@ -323,7 +349,7 @@ function animate(element, options, counterpart, t2, on_finish) { queue_micro_task(() => { if (aborted) return; var o = options({ direction: is_intro ? 'in' : 'out' }); - a = animate(element, o, counterpart, t2, on_finish); + a = animate(element, o, counterpart, t2, on_begin, on_finish); }); // ...but we want to do so without using `async`/`await` everywhere, so @@ -342,7 +368,7 @@ function animate(element, options, counterpart, t2, on_finish) { counterpart?.deactivate(); if (!options?.duration && !options?.delay) { - dispatch_event(element, is_intro ? 'introstart' : 'outrostart'); + on_begin(); on_finish(); return { @@ -382,7 +408,7 @@ function animate(element, options, counterpart, t2, on_finish) { // remove dummy animation from the stack to prevent conflict with main animation animation.cancel(); - dispatch_event(element, is_intro ? 'introstart' : 'outrostart'); + on_begin(); // for bidirectional transitions, we start from the current position, // rather than doing a full intro/outro diff --git a/packages/svelte/tests/runtime-runes/samples/animate-no-transition-events/_config.js b/packages/svelte/tests/runtime-runes/samples/animate-no-transition-events/_config.js new file mode 100644 index 0000000000..3144b53687 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/animate-no-transition-events/_config.js @@ -0,0 +1,31 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, raf, target, logs }) { + let divs = target.querySelectorAll('div'); + divs.forEach((div) => { + // @ts-expect-error + div.getBoundingClientRect = function () { + // @ts-expect-error + const index = [...this.parentNode.children].indexOf(this); + const top = index * 30; + + return { + left: 0, + right: 100, + top, + bottom: top + 20 + }; + }; + }); + + const [btn] = target.querySelectorAll('button'); + flushSync(() => btn.click()); + + raf.tick(1); + assert.deepEqual(logs, []); + raf.tick(100); + assert.deepEqual(logs, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/animate-no-transition-events/main.svelte b/packages/svelte/tests/runtime-runes/samples/animate-no-transition-events/main.svelte new file mode 100644 index 0000000000..a6ba765cf4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/animate-no-transition-events/main.svelte @@ -0,0 +1,19 @@ + + + + +{#each numbers as num (num)} +
console.log("intro start")} + onoutrostart={() => console.log("outro start")} + onintroend={() => console.log("intro end")} + onoutroend={() => console.log("outro end")} + animate:flip={{ duration: 100 }} + > + {num} +
+{/each} From 8e7319063aa609cca2cbf8cdf1958e5392dd2fa0 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 20 Apr 2026 16:35:09 +0200 Subject: [PATCH 2/2] fix: don't mark deriveds while an effect is updating (#18124) Fixes #18123 This makes setting state inside effects slightly slower theoretically (since they hit the new guard), but I verified that the original issue for which we introduced this (#16658) is still fast with this change. The more we add logic to this the more I think we should investigate switching to a different mechanism. I tried using a `Set` previously but it did hurt the benchmarks a bit - might try to revisit a variant of this. --------- Co-authored-by: github-actions[bot] --- .changeset/common-candles-sneeze.md | 5 ++++ .../svelte/src/internal/client/constants.js | 3 +- .../src/internal/client/reactivity/sources.js | 10 +++++-- .../Child.svelte | 7 +++++ .../_config.js | 29 +++++++++++++++++++ .../main.svelte | 14 +++++++++ .../store.svelte.js | 13 +++++++++ 7 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 .changeset/common-candles-sneeze.md create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/store.svelte.js diff --git a/.changeset/common-candles-sneeze.md b/.changeset/common-candles-sneeze.md new file mode 100644 index 0000000000..a0ef7b610b --- /dev/null +++ b/.changeset/common-candles-sneeze.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: don't mark deriveds while an effect is updating diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index a3ad988ba1..f92fba73be 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -48,7 +48,8 @@ export const EFFECT_OFFSCREEN = 1 << 25; /** * Tells that we marked this derived and its reactions as visited during the "mark as (maybe) dirty"-phase. * Will be lifted during execution of the derived and during checking its dirty state (both are necessary - * because a derived might be checked but not executed). + * because a derived might be checked but not executed). This is a pure performance optimization flag and + * should not be used for any other purpose! */ export const WAS_MARKED = 1 << 16; diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 9235c5f673..1831183e6f 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -27,7 +27,8 @@ import { ROOT_EFFECT, ASYNC, WAS_MARKED, - CONNECTED + CONNECTED, + REACTION_IS_UPDATING } from '#client/constants'; import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -356,8 +357,11 @@ function mark_reactions(signal, status, updated_during_traversal) { batch_values?.delete(derived); if ((flags & WAS_MARKED) === 0) { - // Only connected deriveds can be reliably unmarked right away - if (flags & CONNECTED) { + // Only connected deriveds being executed outside the update cycle can be reliably unmarked right away + if ( + flags & CONNECTED && + (active_effect === null || (active_effect.f & REACTION_IS_UPDATING) === 0) + ) { reaction.f |= WAS_MARKED; } diff --git a/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/Child.svelte b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/Child.svelte new file mode 100644 index 0000000000..9771762c0b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/Child.svelte @@ -0,0 +1,7 @@ + diff --git a/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/_config.js b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/_config.js new file mode 100644 index 0000000000..60985cdd12 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/_config.js @@ -0,0 +1,29 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [show, hide] = target.querySelectorAll('button'); + + hide.click(); + flushSync(); + assert.htmlEqual( + target.innerHTML, + ` + + + ` + ); + + show.click(); + flushSync(); + assert.htmlEqual( + target.innerHTML, + ` + + +
visible
+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/main.svelte b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/main.svelte new file mode 100644 index 0000000000..3359a6305b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/main.svelte @@ -0,0 +1,14 @@ + + + + +{#if visible2} + +
visible
+{/if} diff --git a/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/store.svelte.js b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/store.svelte.js new file mode 100644 index 0000000000..b48155e823 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-dep-set-while-rendering/store.svelte.js @@ -0,0 +1,13 @@ +class RawStore { + values = $state.raw({ visible: true }); + + get(key) { + return this.values[key]; + } + + set(key, value) { + this.values = { ...this.values, [key]: value }; + } +} + +export const store = new RawStore();