Skip to content

fix: prevent transition action overfiring #10163

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

Merged
merged 1 commit into from
Jan 11, 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
5 changes: 5 additions & 0 deletions .changeset/spotty-spiders-compare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: prevent transition action overfiring
37 changes: 24 additions & 13 deletions packages/svelte/src/internal/client/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -1375,7 +1375,10 @@ function if_block(anchor_node, condition_fn, consequent_fn, alternate_fn) {
/** @type {null | import('./types.js').TemplateNode | Array<import('./types.js').TemplateNode>} */
let alternate_dom = null;
let has_mounted = false;
let has_mounted_branch = false;
/**
* @type {import("./types.js").EffectSignal | null}
*/
let current_branch_effect = null;

const if_effect = render_effect(
() => {
Expand Down Expand Up @@ -1432,20 +1435,24 @@ function if_block(anchor_node, condition_fn, consequent_fn, alternate_fn) {
);
// Managed effect
const consequent_effect = render_effect(
() => {
if (consequent_dom !== null) {
(
/** @type {any} */ _,
/** @type {import("./types.js").EffectSignal | null} */ consequent_effect
) => {
const result = block.v;
if (!result && consequent_dom !== null) {
remove(consequent_dom);
consequent_dom = null;
}
if (block.v) {
if (result && current_branch_effect !== consequent_effect) {
consequent_fn(anchor_node);
if (!has_mounted_branch) {
if (current_branch_effect === null) {
// Restore previous fragment so that Svelte continues to operate in hydration mode
set_current_hydration_fragment(previous_hydration_fragment);
has_mounted_branch = true;
}
current_branch_effect = consequent_effect;
consequent_dom = block.d;
}
consequent_dom = block.d;
block.d = null;
},
block,
Expand All @@ -1454,22 +1461,26 @@ function if_block(anchor_node, condition_fn, consequent_fn, alternate_fn) {
block.ce = consequent_effect;
// Managed effect
const alternate_effect = render_effect(
() => {
if (alternate_dom !== null) {
(
/** @type {any} */ _,
/** @type {import("./types.js").EffectSignal | null} */ alternate_effect
) => {
const result = block.v;
if (result && alternate_dom !== null) {
remove(alternate_dom);
alternate_dom = null;
}
if (!block.v) {
if (!result && current_branch_effect !== alternate_effect) {
if (alternate_fn !== null) {
alternate_fn(anchor_node);
}
if (!has_mounted_branch) {
if (current_branch_effect === null) {
// Restore previous fragment so that Svelte continues to operate in hydration mode
set_current_hydration_fragment(previous_hydration_fragment);
has_mounted_branch = true;
}
current_branch_effect = alternate_effect;
alternate_dom = block.d;
}
alternate_dom = block.d;
block.d = null;
},
block,
Expand Down
10 changes: 7 additions & 3 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,13 @@ function execute_signal_fn(signal) {
try {
let res;
if (is_render_effect) {
res = /** @type {(block: import('./types.js').Block) => V} */ (init)(
/** @type {import('./types.js').Block} */ (signal.b)
);
res =
/** @type {(block: import('./types.js').Block, signal: import('./types.js').Signal) => V} */ (
init
)(
/** @type {import('./types.js').Block} */ (signal.b),
/** @type {import('./types.js').Signal} */ (signal)
);
} else {
res = /** @type {() => V} */ (init)();
}
Expand Down
27 changes: 12 additions & 15 deletions packages/svelte/src/internal/client/transitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const DELAY_NEXT_TICK = Number.MIN_SAFE_INTEGER;

/** @type {undefined | number} */
let active_tick_ref = undefined;
let skip_mount_intro = false;

/**
* @template T
Expand Down Expand Up @@ -496,7 +495,7 @@ export function bind_transition(dom, get_transition_fn, props_fn, direction, glo
can_show_intro_on_mount = true;
} else if (transition_block.t === IF_BLOCK) {
transition_block.r = if_block_transition;
if (can_show_intro_on_mount && !skip_mount_intro) {
if (can_show_intro_on_mount) {
/** @type {import('./types.js').Block | null} */
let if_block = transition_block;
while (if_block.t === IF_BLOCK) {
Expand All @@ -511,14 +510,13 @@ export function bind_transition(dom, get_transition_fn, props_fn, direction, glo
}
}
if (!can_apply_lazy_transitions && can_show_intro_on_mount) {
can_show_intro_on_mount = !skip_mount_intro && transition_block.e !== null;
can_show_intro_on_mount = transition_block.e !== null;
}
if (can_show_intro_on_mount || !global) {
can_apply_lazy_transitions = true;
}
} else if (transition_block.t === ROOT_BLOCK && !can_apply_lazy_transitions) {
can_show_intro_on_mount =
!skip_mount_intro && (transition_block.e !== null || transition_block.i);
can_show_intro_on_mount = transition_block.e !== null || transition_block.i;
}
transition_block = transition_block.p;
}
Expand All @@ -527,14 +525,11 @@ export function bind_transition(dom, get_transition_fn, props_fn, direction, glo
let transition;

effect(() => {
let already_mounted = false;
if (transition !== undefined) {
already_mounted = true;
// Destroy any existing transitions first
try {
skip_mount_intro = true;
transition.x();
} finally {
skip_mount_intro = false;
}
transition.x();
}
const transition_fn = get_transition_fn();
/** @param {DOMRect} [from] */
Expand All @@ -557,15 +552,15 @@ export function bind_transition(dom, get_transition_fn, props_fn, direction, glo
const is_intro = direction === 'in';
const show_intro = can_show_intro_on_mount && (is_intro || direction === 'both');

if (show_intro) {
if (show_intro && !already_mounted) {
transition.p = transition.i();
}

const effect = managed_pre_effect(() => {
destroy_signal(effect);
dom.inert = false;

if (show_intro) {
if (show_intro && !already_mounted) {
transition.in();
}

Expand Down Expand Up @@ -662,7 +657,8 @@ function if_block_transition(transition) {
const c = /** @type {Set<import('./types.js').Transition>} */ (consequent_transitions);
c.delete(transition);
if (c.size === 0) {
execute_effect(/** @type {import('./types.js').EffectSignal} */ (block.ce));
const consequent_effect = block.ce;
execute_effect(/** @type {import('./types.js').EffectSignal} */ (consequent_effect));
}
});
} else {
Expand All @@ -675,7 +671,8 @@ function if_block_transition(transition) {
const a = /** @type {Set<import('./types.js').Transition>} */ (alternate_transitions);
a.delete(transition);
if (a.size === 0) {
execute_effect(/** @type {import('./types.js').EffectSignal} */ (block.ae));
const alternate_effect = block.ae;
execute_effect(/** @type {import('./types.js').EffectSignal} */ (alternate_effect));
}
});
}
Expand Down
6 changes: 5 additions & 1 deletion packages/svelte/src/internal/client/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,11 @@ export type ComputationSignal<V = unknown> = {
/** The types that the signal represent, as a bitwise value */
f: SignalFlags;
/** init: The function that we invoke for effects and computeds */
i: null | (() => V) | (() => void | (() => void)) | ((b: Block) => void | (() => void));
i:
| null
| (() => V)
| (() => void | (() => void))
| ((b: Block, s: Signal) => void | (() => void));
/** references: Anything that a signal owns */
r: null | ComputationSignal[];
/** value: The latest value for this signal, doubles as the teardown for effects */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ export default test({
b2.click();
});

assert.deepEqual(log, ['transition 2']);

flushSync(() => {
b1.click();
});

assert.deepEqual(log, ['transition 2', 'transition 1']);
}
});