chore: deactivate current_batch by default in unset_context (#17738)

Small tweak: except for `{#await ...}` blocks, which are a bit of an
anomaly, I'm pretty sure we _always_ want to deactivate the current
batch when unsetting context, otherwise it could incorrectly pick up
unrelated state changes. There might even be some subtle bugs lurking in
the system at present because we _don't_ always do this

### Before submitting the PR, please make sure you do the following

- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`.
- [x] This message body should clearly illustrate what problems it
solves.
- [ ] Ideally, include a test that fails without this PR but passes with
it.
- [x] If this PR changes code within `packages/svelte/src`, add a
changeset (`npx changeset`).

### Tests and linting

- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint`
pull/17747/head
Rich Harris 6 days ago committed by GitHub
parent 696d97ff3e
commit f8bf9bb461
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
chore: deactivate current_batch by default in unset_context

@ -12,7 +12,7 @@ import {
import { queue_micro_task } from '../task.js';
import { HYDRATION_START_ELSE, UNINITIALIZED } from '../../../../constants.js';
import { is_runes } from '../../context.js';
import { Batch, flushSync, is_flushing_sync } from '../../reactivity/batch.js';
import { Batch, current_batch, flushSync, is_flushing_sync } from '../../reactivity/batch.js';
import { BranchManager } from './branches.js';
import { capture, unset_context } from '../../reactivity/async.js';
@ -84,7 +84,7 @@ export function await_block(node, get_input, pending_fn, then_fn, catch_fn) {
try {
fn();
} finally {
unset_context();
unset_context(false);
// without this, the DOM does not update until two ticks after the promise
// resolves, which is unexpected behaviour (and somewhat irksome to test)

@ -66,7 +66,6 @@ export function flatten(blockers, sync, async, fn) {
}
}
batch?.deactivate();
unset_context();
}
@ -207,10 +206,11 @@ export async function* for_await_track_reactivity_loss(iterable) {
}
}
export function unset_context() {
export function unset_context(deactivate_batch = true) {
set_active_effect(null);
set_active_reaction(null);
set_component_context(null);
if (deactivate_batch) current_batch?.deactivate();
if (DEV) {
set_from_async_derived(null);
@ -271,9 +271,7 @@ export function run(thunks) {
promise.finally(() => {
blocker.settled = true;
unset_context();
current_batch?.deactivate();
});
}

@ -72,8 +72,6 @@ let is_flushing = false;
export let is_flushing_sync = false;
export class Batch {
committed = false;
/**
* The current values of any sources that are updated in this batch
* They keys of this map are identical to `this.#previous`
@ -425,7 +423,6 @@ export class Batch {
batch_values = previous_batch_values;
}
this.committed = true;
batches.delete(this);
}

@ -133,17 +133,7 @@ export function async_derived(fn, label, location) {
// If this code is changed at some point, make sure to still access the then property
// of fn() to read any signals it might access, so that we track them as dependencies.
// We call `unset_context` to undo any `save` calls that happen inside `fn()`
Promise.resolve(fn())
.then(d.resolve, d.reject)
.then(() => {
if (batch === current_batch && batch.committed) {
// if the batch was rejected as stale, we need to cleanup
// after any `$.save(...)` calls inside `fn()`
batch.deactivate();
}
unset_context();
});
Promise.resolve(fn()).then(d.resolve, d.reject).finally(unset_context);
} catch (error) {
d.reject(error);
unset_context();

Loading…
Cancel
Save