Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
64f078d
chore: remove unowned check when calling `e.effect_in_unowned_derived`
Rich-Harris Nov 3, 2025
c9d26db
WIP
Rich-Harris Nov 3, 2025
db21188
all non-unit tests passing
Rich-Harris Nov 3, 2025
b5bb6fb
tidy
Rich-Harris Nov 3, 2025
625d289
WIP
Rich-Harris Nov 3, 2025
6700adb
WIP
Rich-Harris Nov 3, 2025
697fba0
WIP
Rich-Harris Nov 3, 2025
6eb4dee
note to self
Rich-Harris Nov 3, 2025
93f277d
fix
Rich-Harris Nov 3, 2025
e1fbee5
fix
Rich-Harris Nov 3, 2025
b61d1b0
hmm maybe not
Rich-Harris Nov 3, 2025
5b42362
Merge branch 'main' into remove-unowned
Rich-Harris Nov 3, 2025
15f9f85
try this
Rich-Harris Nov 4, 2025
67f544e
simplify
Rich-Harris Nov 4, 2025
d1149d6
remove skip_reaction
Rich-Harris Nov 4, 2025
d5c7605
docs
Rich-Harris Nov 4, 2025
0bd13a9
add changeset, in case this results in changed behaviour
Rich-Harris Nov 4, 2025
3197022
Update packages/svelte/src/internal/client/reactivity/effects.js
Rich-Harris Nov 4, 2025
4d5209c
fix #17024
Rich-Harris Nov 4, 2025
b8fc364
fix comment
Rich-Harris Nov 4, 2025
3d4fa95
revert
Rich-Harris Nov 4, 2025
027194a
fix
Rich-Harris Nov 4, 2025
69290af
dry
Rich-Harris Nov 4, 2025
c96eb44
changeset
Rich-Harris Nov 4, 2025
1032b31
fix WAS_MARKED logic
dummdidumm Nov 4, 2025
22435b2
failing test (that uncovered other unrelated bug) + fix
dummdidumm Nov 5, 2025
d55107f
fix: delete from batch_values on updates (#17115)
dummdidumm Nov 5, 2025
df077f3
tidy up
Rich-Harris Nov 5, 2025
66d6d4b
Merge branch 'main' into remove-unowned
Rich-Harris Nov 5, 2025
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
3 changes: 1 addition & 2 deletions packages/svelte/src/internal/client/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ export const EFFECT_PRESERVED = 1 << 19;
export const USER_EFFECT = 1 << 20;

// Flags exclusive to deriveds
export const UNOWNED = 1 << 8;
export const DISCONNECTED = 1 << 9;
export const CONNECTED = 1 << 9;
/**
* 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
Expand Down
13 changes: 4 additions & 9 deletions packages/svelte/src/internal/client/reactivity/deriveds.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@ import {
EFFECT_PRESERVED,
MAYBE_DIRTY,
STALE_REACTION,
UNOWNED,
ASYNC,
WAS_MARKED
WAS_MARKED,
CONNECTED
} from '#client/constants';
import {
active_reaction,
active_effect,
set_signal_status,
skip_reaction,
update_reaction,
increment_write_version,
set_active_effect,
Expand Down Expand Up @@ -61,9 +60,7 @@ export function derived(fn) {
? /** @type {Derived} */ (active_reaction)
: null;

if (active_effect === null || (parent_derived !== null && (parent_derived.f & UNOWNED) !== 0)) {
flags |= UNOWNED;
} else {
if (active_effect !== null) {
// Since deriveds are evaluated lazily, any effects created inside them are
// created too late to ensure that the parent effect is added to the tree
active_effect.f |= EFFECT_PRESERVED;
Expand Down Expand Up @@ -371,9 +368,7 @@ export function update_derived(derived) {
if (batch_values !== null) {
batch_values.set(derived, derived.v);
} else {
var status =
(skip_reaction || (derived.f & UNOWNED) !== 0) && derived.deps !== null ? MAYBE_DIRTY : CLEAN;

var status = (derived.f & CONNECTED) === 0 ? MAYBE_DIRTY : CLEAN;
set_signal_status(derived, status);
}
}
12 changes: 6 additions & 6 deletions packages/svelte/src/internal/client/reactivity/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ import {
ROOT_EFFECT,
EFFECT_TRANSPARENT,
DERIVED,
UNOWNED,
CLEAN,
EAGER_EFFECT,
HEAD_EFFECT,
MAYBE_DIRTY,
EFFECT_PRESERVED,
STALE_REACTION,
USER_EFFECT,
ASYNC
ASYNC,
CONNECTED
} from '#client/constants';
import * as e from '../errors.js';
import { DEV } from 'esm-env';
Expand All @@ -49,10 +49,10 @@ import { without_reactive_context } from '../dom/elements/bindings/shared.js';
*/
export function validate_effect(rune) {
if (active_effect === null && active_reaction === null) {
e.effect_orphan(rune);
}
if (active_reaction === null) {
e.effect_orphan(rune);
}

if (active_reaction !== null && (active_reaction.f & UNOWNED) !== 0 && active_effect === null) {
e.effect_in_unowned_derived();
}

Expand Down Expand Up @@ -103,7 +103,7 @@ function create_effect(type, fn, sync, push = true) {
deps: null,
nodes_start: null,
nodes_end: null,
f: type | DIRTY,
f: type | DIRTY | CONNECTED,
first: null,
fn,
last: null,
Expand Down
7 changes: 4 additions & 3 deletions packages/svelte/src/internal/client/reactivity/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ import {
DIRTY,
BRANCH_EFFECT,
EAGER_EFFECT,
UNOWNED,
MAYBE_DIRTY,
BLOCK_EFFECT,
ROOT_EFFECT,
ASYNC,
WAS_MARKED
WAS_MARKED,
CONNECTED
} from '#client/constants';
import * as e from '../errors.js';
import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js';
Expand Down Expand Up @@ -211,7 +211,8 @@ export function internal_set(source, value) {
if ((source.f & DIRTY) !== 0) {
execute_derived(/** @type {Derived} */ (source));
}
set_signal_status(source, (source.f & UNOWNED) === 0 ? CLEAN : MAYBE_DIRTY);

set_signal_status(source, (source.f & CONNECTED) !== 0 ? CLEAN : MAYBE_DIRTY);
}

source.wv = increment_write_version();
Expand Down
98 changes: 32 additions & 66 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@ import { get_descriptors, get_prototype_of, index_of } from '../shared/utils.js'
import {
destroy_block_effect_children,
destroy_effect_children,
effect_tracking,
execute_effect_teardown
} from './reactivity/effects.js';
import {
DIRTY,
MAYBE_DIRTY,
CLEAN,
DERIVED,
UNOWNED,
DESTROYED,
BRANCH_EFFECT,
STATE_SYMBOL,
BLOCK_EFFECT,
ROOT_EFFECT,
DISCONNECTED,
CONNECTED,
REACTION_IS_UPDATING,
STALE_REACTION,
ERROR_VALUE,
Expand Down Expand Up @@ -139,7 +139,7 @@ export function set_update_version(value) {

// If we are working with a get() chain that has no active container,
// to prevent memory leaks, we skip adding the reaction.
export let skip_reaction = false;
let skip_reaction = false;

export function increment_write_version() {
return ++write_version;
Expand All @@ -160,7 +160,6 @@ export function is_dirty(reaction) {

if ((flags & MAYBE_DIRTY) !== 0) {
var dependencies = reaction.deps;
var is_unowned = (flags & UNOWNED) !== 0;

if (flags & DERIVED) {
reaction.f &= ~WAS_MARKED;
Expand All @@ -169,42 +168,8 @@ export function is_dirty(reaction) {
if (dependencies !== null) {
var i;
var dependency;
var is_disconnected = (flags & DISCONNECTED) !== 0;
var is_unowned_connected = is_unowned && active_effect !== null && !skip_reaction;
var length = dependencies.length;

// If we are working with a disconnected or an unowned signal that is now connected (due to an active effect)
// then we need to re-connect the reaction to the dependency, unless the effect has already been destroyed
// (which can happen if the derived is read by an async derived)
if (
(is_disconnected || is_unowned_connected) &&
(active_effect === null || (active_effect.f & DESTROYED) === 0)
) {
var derived = /** @type {Derived} */ (reaction);
var parent = derived.parent;

for (i = 0; i < length; i++) {
dependency = dependencies[i];

// We always re-add all reactions (even duplicates) if the derived was
// previously disconnected, however we don't if it was unowned as we
// de-duplicate dependencies in that case
if (is_disconnected || !dependency?.reactions?.includes(derived)) {
(dependency.reactions ??= []).push(derived);
}
}

if (is_disconnected) {
derived.f ^= DISCONNECTED;
}
// If the unowned derived is now fully connected to the graph again (it's unowned and reconnected, has a parent
// and the parent is not unowned), then we can mark it as connected again, removing the need for the unowned
// flag
if (is_unowned_connected && parent !== null && (parent.f & UNOWNED) === 0) {
derived.f ^= UNOWNED;
}
}

for (i = 0; i < length; i++) {
dependency = dependencies[i];

Expand All @@ -218,9 +183,7 @@ export function is_dirty(reaction) {
}
}

// Unowned signals should never be marked as clean unless they
// are used within an active_effect without skip_reaction
if (!is_unowned || (active_effect !== null && !skip_reaction)) {
if ((flags & CONNECTED) !== 0) {
set_signal_status(reaction, CLEAN);
}
}
Expand Down Expand Up @@ -274,8 +237,7 @@ export function update_reaction(reaction) {
new_deps = /** @type {null | Value[]} */ (null);
skipped_deps = 0;
untracked_writes = null;
skip_reaction =
(flags & UNOWNED) !== 0 && (untracking || !is_updating_effect || active_reaction === null);
skip_reaction = !effect_tracking();
active_reaction = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ? reaction : null;

current_sources = null;
Expand Down Expand Up @@ -311,12 +273,7 @@ export function update_reaction(reaction) {
reaction.deps = deps = new_deps;
}

if (
!skip_reaction ||
// Deriveds that already have reactions can cleanup, so we still add them as reactions
((flags & DERIVED) !== 0 &&
/** @type {import('#client').Derived} */ (reaction).reactions !== null)
) {
if (is_updating_effect && effect_tracking() && (reaction.f & CONNECTED) !== 0) {
for (i = skipped_deps; i < deps.length; i++) {
(deps[i].reactions ??= []).push(reaction);
}
Expand Down Expand Up @@ -416,8 +373,8 @@ function remove_reaction(signal, dependency) {
set_signal_status(dependency, MAYBE_DIRTY);
// If we are working with a derived that is owned by an effect, then mark it as being
// disconnected.
if ((dependency.f & (UNOWNED | DISCONNECTED)) === 0) {
dependency.f ^= DISCONNECTED;
if ((dependency.f & CONNECTED) !== 0) {
dependency.f ^= CONNECTED;
}
// Disconnect any reactions owned by this reaction
destroy_derived_effects(/** @type {Derived} **/ (dependency));
Expand Down Expand Up @@ -585,20 +542,6 @@ export function get(signal) {
}
}
}
} else if (
is_derived &&
/** @type {Derived} */ (signal).deps === null &&
/** @type {Derived} */ (signal).effects === null
) {
var derived = /** @type {Derived} */ (signal);
var parent = derived.parent;

if (parent !== null && (parent.f & UNOWNED) === 0) {
// If the derived is owned by another derived then mark it as unowned
// as the derived value might have been referenced in a different context
// since and thus its parent might not be its true owner anymore
derived.f ^= UNOWNED;
}
}

if (DEV) {
Expand Down Expand Up @@ -657,7 +600,7 @@ export function get(signal) {
}

if (is_derived) {
derived = /** @type {Derived} */ (signal);
var derived = /** @type {Derived} */ (signal);

var value = derived.v;

Expand All @@ -684,6 +627,10 @@ export function get(signal) {
if (is_dirty(derived)) {
update_derived(derived);
}

if (is_updating_effect && effect_tracking() && (derived.f & CONNECTED) === 0) {
reconnect(derived);
}
}

if (batch_values?.has(signal)) {
Expand All @@ -697,6 +644,25 @@ export function get(signal) {
return signal.v;
}

/**
* (Re)connect a disconnected derived, so that it is notified
* of changes in `mark_reactions`
* @param {Derived} derived
*/
function reconnect(derived) {
if (derived.deps === null) return;

derived.f ^= CONNECTED;

for (const dep of derived.deps) {
(dep.reactions ??= []).push(derived);

if ((dep.f & DERIVED) !== 0 && (dep.f & CONNECTED) === 0) {
reconnect(/** @type {Derived} */ (dep));
}
}
}

/** @param {Derived} derived */
function depends_on_old_values(derived) {
if (derived.v === UNINITIALIZED) return true; // we don't know, so assume the worst
Expand Down
Loading