Skip to content

fix: make untrack behave correctly in relation to mutations #14784

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
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/strong-cows-jump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: make untrack behave correctly in relation to mutations
3 changes: 2 additions & 1 deletion packages/svelte/src/index-client.js
Original file line number Diff line number Diff line change
@@ -187,7 +187,8 @@ export {
hasContext,
setContext,
tick,
untrack
untrack,
unsafe
} from './internal/client/runtime.js';

export { createRawSnippet } from './internal/client/dom/blocks/snippet.js';
3 changes: 2 additions & 1 deletion packages/svelte/src/index-server.js
Original file line number Diff line number Diff line change
@@ -14,7 +14,8 @@ export {
noop as afterUpdate,
noop as onMount,
noop as flushSync,
run as untrack
run as untrack,
run as unsafe
} from './internal/shared/utils.js';

export function createEventDispatcher() {
5 changes: 3 additions & 2 deletions packages/svelte/src/internal/client/reactivity/effects.js
Original file line number Diff line number Diff line change
@@ -16,7 +16,8 @@ import {
set_is_flushing_effect,
set_signal_status,
untrack,
skip_reaction
skip_reaction,
untracking
} from '../runtime.js';
import {
DIRTY,
@@ -164,7 +165,7 @@ function create_effect(type, fn, sync, push = true) {
* @returns {boolean}
*/
export function effect_tracking() {
if (active_reaction === null) {
if (active_reaction === null || untracking) {
return false;
}

31 changes: 23 additions & 8 deletions packages/svelte/src/internal/client/reactivity/sources.js
Original file line number Diff line number Diff line change
@@ -18,7 +18,10 @@ import {
set_derived_sources,
check_dirtiness,
set_is_flushing_effect,
is_flushing_effect
is_flushing_effect,
unsafe_mutating,
unsafe_sources,
set_unsafe_sources
} from '../runtime.js';
import { equals, safe_equals } from './equality.js';
import {
@@ -147,6 +150,7 @@ export function mutate(source, value) {
export function set(source, value) {
if (
active_reaction !== null &&
!unsafe_mutating &&
is_runes() &&
(active_reaction.f & (DERIVED | BLOCK_EFFECT)) !== 0 &&
// If the source was created locally within the current derived, then
@@ -170,6 +174,14 @@ export function internal_set(source, value) {
source.v = value;
source.version = increment_version();

if (unsafe_mutating) {
if (unsafe_sources === null) {
set_unsafe_sources([source]);
} else {
unsafe_sources.push(source);
}
}

if (DEV && tracing_mode_flag) {
source.updated = get_stack('UpdatedAt');
}
@@ -227,9 +239,10 @@ export function internal_set(source, value) {
/**
* @param {Value} signal
* @param {number} status should be DIRTY or MAYBE_DIRTY
* @param {boolean} [unsafe] mark all reactions for unsafe mutations
* @returns {void}
*/
function mark_reactions(signal, status) {
export function mark_reactions(signal, status, unsafe = false) {
var reactions = signal.reactions;
if (reactions === null) return;

@@ -241,23 +254,25 @@ function mark_reactions(signal, status) {
var flags = reaction.f;

// Skip any effects that are already dirty
if ((flags & DIRTY) !== 0) continue;
if ((flags & DIRTY) !== 0 && !unsafe) continue;

// In legacy mode, skip the current effect to prevent infinite loops
if (!runes && reaction === active_effect) continue;

// Inspect effects need to run immediately, so that the stack trace makes sense
if (DEV && (flags & INSPECT_EFFECT) !== 0) {
// Inspect effects need to run immediately, so that the stack trace makes sense.
// Skip doing this for the unsafe mutations as they will have already been added
// in the unsafe() wrapper
if (DEV && !unsafe && (flags & INSPECT_EFFECT) !== 0) {
inspect_effects.add(reaction);
continue;
}

set_signal_status(reaction, status);

// If the signal a) was previously clean or b) is an unowned derived, then mark it
if ((flags & (CLEAN | UNOWNED)) !== 0) {
// If the signal a) was previously clean or b) is an unowned derived then mark it
if ((flags & (CLEAN | UNOWNED)) !== 0 || unsafe) {
if ((flags & DERIVED) !== 0) {
mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY);
mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY, unsafe);
} else {
schedule_effect(/** @type {Effect} */ (reaction));
}
59 changes: 54 additions & 5 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@ import {
} from './constants.js';
import { flush_tasks } from './dom/task.js';
import { add_owner } from './dev/ownership.js';
import { internal_set, set, source } from './reactivity/sources.js';
import { internal_set, mark_reactions, set, source } from './reactivity/sources.js';
import { destroy_derived, execute_derived, update_derived } from './reactivity/deriveds.js';
import * as e from './errors.js';
import { lifecycle_outside_component } from '../shared/errors.js';
@@ -78,6 +78,18 @@ let dev_effect_stack = [];
/** @type {null | Reaction} */
export let active_reaction = null;

export let untracking = false;

export let unsafe_mutating = false;

/** @type {null | Source[]} */
export let unsafe_sources = null;

/** @param {Source[] | null} value */
export function set_unsafe_sources(value) {
unsafe_sources = value;
}

/** @param {null | Reaction} reaction */
export function set_active_reaction(reaction) {
active_reaction = reaction;
@@ -387,6 +399,8 @@ export function update_reaction(reaction) {
var previous_skip_reaction = skip_reaction;
var prev_derived_sources = derived_sources;
var previous_component_context = component_context;
var previous_untracking = untracking;
var previous_unsafe_mutating = unsafe_mutating;
var flags = reaction.f;

new_deps = /** @type {null | Value[]} */ (null);
@@ -396,6 +410,8 @@ export function update_reaction(reaction) {
skip_reaction = !is_flushing_effect && (flags & UNOWNED) !== 0;
derived_sources = null;
component_context = reaction.ctx;
untracking = false;
unsafe_mutating = false;

try {
var result = /** @type {Function} */ (0, reaction.fn)();
@@ -434,6 +450,8 @@ export function update_reaction(reaction) {
skip_reaction = previous_skip_reaction;
derived_sources = prev_derived_sources;
component_context = previous_component_context;
untracking = previous_untracking;
unsafe_mutating = previous_unsafe_mutating;
}
}

@@ -507,8 +525,10 @@ export function update_effect(effect) {

var previous_effect = active_effect;
var previous_component_context = component_context;
var previous_unsafe_sources = unsafe_sources;

active_effect = effect;
unsafe_sources = null;

if (DEV) {
var previous_component_fn = dev_current_component_function;
@@ -526,6 +546,16 @@ export function update_effect(effect) {
execute_effect_teardown(effect);
var teardown = update_reaction(effect);
effect.teardown = typeof teardown === 'function' ? teardown : null;

// If unsafe() has been used within the effect then we will need
// to re-invalidate any unsafe sources that were already invalidated
// to ensure consistency of the graph
if (unsafe_sources !== null && (effect.f & CLEAN) !== 0) {
for (let i = 0; i < /** @type {Source[]} */ (unsafe_sources).length; i++) {
mark_reactions(unsafe_sources[i], DIRTY, true);
}
}

effect.version = current_version;

if (DEV) {
@@ -535,6 +565,7 @@ export function update_effect(effect) {
handle_error(error, effect, previous_effect, previous_component_context || effect.ctx);
} finally {
active_effect = previous_effect;
unsafe_sources = previous_unsafe_sources;

if (DEV) {
dev_current_component_function = previous_component_fn;
@@ -856,7 +887,7 @@ export function get(signal) {
}

// Register the dependency on the current reaction signal.
if (active_reaction !== null) {
if (active_reaction !== null && !untracking) {
if (derived_sources !== null && derived_sources.includes(signal)) {
e.state_unsafe_local_read();
}
@@ -1016,12 +1047,30 @@ export function invalidate_inner_signals(fn) {
* @returns {T}
*/
export function untrack(fn) {
const previous_reaction = active_reaction;
var previous_untracking = untracking;
try {
active_reaction = null;
untracking = true;
return fn();
} finally {
active_reaction = previous_reaction;
untracking = previous_untracking;
}
}

/**
* When used inside a [`$derived`](https://svelte.dev/docs/svelte/$derived),
* any state updates to state is allowed.
*
* @template T
* @param {() => T} fn
* @returns {T}
*/
export function unsafe(fn) {
var previous_unsafe_mutating = unsafe_mutating;
try {
unsafe_mutating = true;
return fn();
} finally {
unsafe_mutating = previous_unsafe_mutating;
}
}

4 changes: 2 additions & 2 deletions packages/svelte/src/store/utils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @import { Readable } from './public' */
import { untrack } from '../index-client.js';
import { unsafe } from '../index-client.js';
import { noop } from '../internal/shared/utils.js';

/**
@@ -22,7 +22,7 @@ export function subscribe_to_store(store, run, invalidate) {

// Svelte store takes a private second argument
// StartStopNotifier could mutate state, and we want to silence the corresponding validation error
const unsub = untrack(() =>
const unsub = unsafe(() =>
store.subscribe(
run,
// @ts-expect-error
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script>
import { untrack } from 'svelte';
import { unsafe } from 'svelte';
import { SvelteMap } from 'svelte/reactivity';

const cache = new SvelteMap();
@@ -13,7 +13,7 @@
cache.set(id, id.toString());
}).then(() => cache.get(id));

untrack(() => {
unsafe(() => {
cache.set(id, promise);
});

@@ -25,7 +25,7 @@

const value = $derived(get_async(1));
const value2 = $derived(get_async(1));
// both values are read before the set
// both values are read before the set
value;
value2;
</script>
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<script>
import { untrack } from 'svelte';
import { unsafe, untrack } from 'svelte';

let count = $state(0);
function default_arg() {
untrack(() => count++);
untrack(() => unsafe(() => count++));
return 1;
}
</script>
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<script>
import { untrack } from 'svelte';
import { unsafe, untrack } from 'svelte';

let count = $state(0);
function default_arg() {
untrack(() => count++);
untrack(() => unsafe(() => count++));
return 1;
}
</script>
Loading
Loading