Skip to content

fix: ensure untrack correctly retains the active reaction #15065

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 2 commits into from
Jan 21, 2025
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/fresh-cycles-sneeze.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: ensure untrack correctly retains the active reaction
8 changes: 4 additions & 4 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 All @@ -43,8 +44,7 @@ import * as e from '../errors.js';
import { DEV } from 'esm-env';
import { define_property } from '../../shared/utils.js';
import { get_next_sibling } from '../dom/operations.js';
import { derived, derived_safe_equal, destroy_derived } from './deriveds.js';
import { legacy_mode_flag } from '../../flags/index.js';
import { derived, destroy_derived } from './deriveds.js';

/**
* @param {'$effect' | '$effect.pre' | '$inspect'} rune
Expand Down Expand Up @@ -166,7 +166,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
4 changes: 3 additions & 1 deletion packages/svelte/src/internal/client/reactivity/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import {
set_derived_sources,
check_dirtiness,
set_is_flushing_effect,
is_flushing_effect
is_flushing_effect,
untracking
} from '../runtime.js';
import { equals, safe_equals } from './equality.js';
import {
Expand Down Expand Up @@ -148,6 +149,7 @@ export function mutate(source, value) {
export function set(source, value) {
if (
active_reaction !== null &&
!untracking &&
is_runes() &&
(active_reaction.f & (DERIVED | BLOCK_EFFECT)) !== 0 &&
// If the source was created locally within the current derived, then
Expand Down
13 changes: 9 additions & 4 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ let dev_effect_stack = [];
/** @type {null | Reaction} */
export let active_reaction = null;

export let untracking = false;

/** @param {null | Reaction} reaction */
export function set_active_reaction(reaction) {
active_reaction = reaction;
Expand Down Expand Up @@ -423,6 +425,7 @@ 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 flags = reaction.f;

new_deps = /** @type {null | Value[]} */ (null);
Expand All @@ -432,6 +435,7 @@ export function update_reaction(reaction) {
skip_reaction = !is_flushing_effect && (flags & UNOWNED) !== 0;
derived_sources = null;
component_context = reaction.ctx;
untracking = false;
read_version++;

try {
Expand Down Expand Up @@ -495,6 +499,7 @@ export function update_reaction(reaction) {
skip_reaction = previous_skip_reaction;
derived_sources = prev_derived_sources;
component_context = previous_component_context;
untracking = previous_untracking;
}
}

Expand Down Expand Up @@ -934,7 +939,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 @@ -1085,12 +1090,12 @@ 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;
}
}

Expand Down
40 changes: 40 additions & 0 deletions packages/svelte/tests/signals/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,46 @@ describe('signals', () => {
};
});

test('deriveds containing effects work correctly when used with untrack', () => {
return () => {
let a = render_effect(() => {});
let b = state(0);
let c;
let effects = [];

const destroy = effect_root(() => {
a = render_effect(() => {
c = derived(() => {
$.untrack(() => {
effects.push(
effect(() => {
$.get(b);
})
);
});
$.get(b);
});
$.get(c);
});
});

assert.deepEqual(c!.children?.length, 1);
assert.deepEqual(a.first, a.last);

set(b, 1);

flushSync();

assert.deepEqual(c!.children?.length, 1);
assert.deepEqual(a.first, a.last);

destroy();

assert.deepEqual(a.deriveds, null);
assert.deepEqual(a.first, null);
};
});

test('bigint states update correctly', () => {
return () => {
const count = state(0n);
Expand Down
Loading