Skip to content

Commit

Permalink
avoid eagerly trigger user effects or templates effects when suspended
Browse files Browse the repository at this point in the history
  • Loading branch information
trueadm committed Jan 24, 2025
1 parent e102ec0 commit debc148
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 30 deletions.
31 changes: 17 additions & 14 deletions packages/svelte/src/internal/client/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,26 @@ export const BLOCK_EFFECT = 1 << 4;
export const BRANCH_EFFECT = 1 << 5;
export const ROOT_EFFECT = 1 << 6;
export const BOUNDARY_EFFECT = 1 << 7;
export const UNOWNED = 1 << 8;
export const DISCONNECTED = 1 << 9;
export const CLEAN = 1 << 10;
export const DIRTY = 1 << 11;
export const MAYBE_DIRTY = 1 << 12;
export const INERT = 1 << 13;
export const DESTROYED = 1 << 14;
export const EFFECT_RAN = 1 << 15;
export const TEMPLATE_EFFECT = 1 << 8;
export const UNOWNED = 1 << 9;
export const DISCONNECTED = 1 << 10;
export const CLEAN = 1 << 11;
export const DIRTY = 1 << 12;
export const MAYBE_DIRTY = 1 << 13;
export const INERT = 1 << 14;
export const DESTROYED = 1 << 15;
export const EFFECT_RAN = 1 << 16;
/** 'Transparent' effects do not create a transition boundary */
export const EFFECT_TRANSPARENT = 1 << 16;
export const EFFECT_TRANSPARENT = 1 << 17;
/** Svelte 4 legacy mode props need to be handled with deriveds and be recognized elsewhere, hence the dedicated flag */
export const LEGACY_DERIVED_PROP = 1 << 17;
export const INSPECT_EFFECT = 1 << 18;
export const HEAD_EFFECT = 1 << 19;
export const EFFECT_HAS_DERIVED = 1 << 20;
export const LEGACY_DERIVED_PROP = 1 << 18;
export const INSPECT_EFFECT = 1 << 19;
export const HEAD_EFFECT = 1 << 20;
export const EFFECT_HAS_DERIVED = 1 << 21;

export const REACTION_IS_UPDATING = 1 << 21;
// Flags used for async
export const IS_ASYNC = 1 << 22;
export const REACTION_IS_UPDATING = 1 << 23;

export const STATE_SYMBOL = Symbol('$state');
export const STATE_SYMBOL_METADATA = Symbol('$state metadata');
Expand Down
3 changes: 2 additions & 1 deletion packages/svelte/src/internal/client/reactivity/deriveds.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
DESTROYED,
DIRTY,
EFFECT_HAS_DERIVED,
IS_ASYNC,
MAYBE_DIRTY,
UNOWNED
} from '../constants.js';
Expand Down Expand Up @@ -158,7 +159,7 @@ export function async_derived(fn) {
// TODO we should probably null out active effect here,
// rather than inside `restore()`
}
}, EFFECT_HAS_DERIVED);
}, IS_ASYNC);

return promise.then(() => value);
}
Expand Down
8 changes: 5 additions & 3 deletions packages/svelte/src/internal/client/reactivity/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ import {
HEAD_EFFECT,
MAYBE_DIRTY,
EFFECT_HAS_DERIVED,
BOUNDARY_EFFECT
BOUNDARY_EFFECT,
IS_ASYNC,
TEMPLATE_EFFECT
} from '../constants.js';
import { set } from './sources.js';
import * as e from '../errors.js';
Expand Down Expand Up @@ -145,7 +147,7 @@ function create_effect(type, fn, sync, push = true) {
effect.first === null &&
effect.nodes_start === null &&
effect.teardown === null &&
(effect.f & (EFFECT_HAS_DERIVED | BOUNDARY_EFFECT)) === 0;
(effect.f & (EFFECT_HAS_DERIVED | BOUNDARY_EFFECT | IS_ASYNC)) === 0;

if (!inert && !is_root && push) {
if (parent_effect !== null) {
Expand Down Expand Up @@ -385,7 +387,7 @@ function create_template_effect(fn, deriveds) {
});
}

block(effect);
block(effect, TEMPLATE_EFFECT);
}

/**
Expand Down
26 changes: 23 additions & 3 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import {
LEGACY_DERIVED_PROP,
DISCONNECTED,
BOUNDARY_EFFECT,
REACTION_IS_UPDATING
REACTION_IS_UPDATING,
IS_ASYNC,
TEMPLATE_EFFECT
} from './constants.js';
import {
flush_idle_tasks,
Expand Down Expand Up @@ -102,6 +104,7 @@ export function set_active_effect(effect) {
/* @__PURE__ */
setInterval(() => {
if (active_effect !== null || active_reaction !== null) {
// eslint-disable-next-line no-debugger
debugger;
}
});
Expand Down Expand Up @@ -819,6 +822,7 @@ export function schedule_effect(signal) {
function process_effects(effect, collected_effects) {
var current_effect = effect.first;
var effects = [];
var suspended = false;

main_loop: while (current_effect !== null) {
var flags = current_effect.f;
Expand All @@ -827,13 +831,25 @@ function process_effects(effect, collected_effects) {
var sibling = current_effect.next;

if (!is_skippable_branch && (flags & INERT) === 0) {
var skip_suspended =
suspended &&
(flags & BRANCH_EFFECT) === 0 &&
((flags & BLOCK_EFFECT) === 0 || (flags & TEMPLATE_EFFECT) !== 0);

if ((flags & RENDER_EFFECT) !== 0) {
if (is_branch) {
current_effect.f ^= CLEAN;
} else {
} else if (!skip_suspended) {
try {
var is_async_effect = (current_effect.f & IS_ASYNC) !== 0;

if (check_dirtiness(current_effect)) {
update_effect(current_effect);
if (!suspended && is_async_effect) {
suspended = true;
}
} else if (!suspended && is_async_effect && current_effect.deps === null) {
suspended = true;
}
} catch (error) {
handle_error(error, current_effect, null, current_effect.ctx);
Expand All @@ -846,7 +862,7 @@ function process_effects(effect, collected_effects) {
current_effect = child;
continue;
}
} else if ((flags & EFFECT) !== 0) {
} else if ((flags & EFFECT) !== 0 && !skip_suspended) {
effects.push(current_effect);
}
}
Expand All @@ -858,6 +874,10 @@ function process_effects(effect, collected_effects) {
if (effect === parent) {
break main_loop;
}
// TODO: we need to know that this boundary has a valid `pending`
if (suspended && (parent.f & BOUNDARY_EFFECT) !== 0) {
suspended = false;
}
var parent_sibling = parent.next;
if (parent_sibling !== null) {
current_effect = parent_sibling;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export default test({
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
await tick();
flushSync();
assert.htmlEqual(target.innerHTML, '<p>42</p>');

Expand All @@ -34,7 +33,6 @@ export default test({
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
await tick();
assert.htmlEqual(target.innerHTML, '<p>84</p>');

Expand All @@ -47,20 +45,18 @@ export default test({
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
await tick();
assert.htmlEqual(target.innerHTML, '<p>86</p>');

assert.deepEqual(logs, [
'outside boundary 1',
'$effect.pre 42 1',
'template 42 1',
'$effect 42 1',
'$effect.pre 42 2',
'template 42 2',
'$effect 42 2',
'$effect.pre 84 2',
'template 84 2',
'$effect 84 2',
'outside boundary 2',
'$effect.pre 84 2', // TODO: why is this observed during tests, but not during runtime?
'template 84 2', // TODO: why is this observed during tests, but not during runtime?
'$effect 84 2', // TODO: why is this observed during tests, but not during runtime?
'$effect.pre 86 2',
'template 86 2',
'$effect 86 2'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@
<p>pending</p>
{/snippet}
</svelte:boundary>

{console.log(`outside boundary ${num}`)}

0 comments on commit debc148

Please sign in to comment.