Skip to content
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

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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/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
Expand Up @@ -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
Expand Up @@ -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() {
Expand Down
5 changes: 3 additions & 2 deletions packages/svelte/src/internal/client/reactivity/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
set_is_flushing_effect,
set_signal_status,
untrack,
skip_reaction
skip_reaction,
untracking
} from '../runtime.js';
import {
DIRTY,
Expand Down Expand Up @@ -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;
}

Expand Down
31 changes: 23 additions & 8 deletions packages/svelte/src/internal/client/reactivity/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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');
}
Expand Down Expand Up @@ -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;

Expand All @@ -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));
}
Expand Down
59 changes: 54 additions & 5 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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)();
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
}
}

Expand Down
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';

/**
Expand All @@ -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
Expand Down
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();
Expand All @@ -13,7 +13,7 @@
cache.set(id, id.toString());
}).then(() => cache.get(id));

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

Expand All @@ -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>
Expand Down
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>
Expand Down
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>
Expand Down
Loading
Loading