tweak logic about (not) using batch_values

async-derived-coordinate-batches
Simon Holthausen 1 month ago
parent 03fa759b80
commit 5ca1c72134

@ -61,6 +61,26 @@ export let previous_batch = null;
*/
export let batch_values = null;
/** @type {boolean | null} */
export let has_batch_value_differences = null;
/** @type {boolean} */
export let ignore_batch_values = false;
/**
* @param {boolean | null} value
*/
export function set_has_batch_value_differences(value) {
has_batch_value_differences = value;
}
/**
* @param {boolean} value
*/
export function set_ignore_batch_values(value) {
ignore_batch_values = value;
}
/** @type {Effect | null} */
let last_scheduled_effect = null;
@ -228,6 +248,7 @@ export class Batch {
const roots = this.#roots;
this.#roots = [];
// TODO if we move this below traverse only one test fails - is this good (close to being able to do this) or bad (not enough test coverage)?
this.apply();
/** @type {Effect[]} */
@ -341,9 +362,21 @@ export class Batch {
effects.push(effect);
} else if (async_mode_flag && (flags & (RENDER_EFFECT | MANAGED_EFFECT)) !== 0) {
render_effects.push(effect);
} else if (is_dirty(effect)) {
if ((flags & BLOCK_EFFECT) !== 0) this.#maybe_dirty_effects.add(effect);
update_effect(effect);
} else {
// ASYNC effects should read canonical signal values instead of batch overlays
// while they run.
var previous_ignore_batch_values = ignore_batch_values;
if ((flags & ASYNC) !== 0) {
set_ignore_batch_values(true);
}
try {
if (is_dirty(effect)) {
if ((flags & BLOCK_EFFECT) !== 0) this.#maybe_dirty_effects.add(effect);
update_effect(effect);
}
} finally {
set_ignore_batch_values(previous_ignore_batch_values);
}
}
var child = effect.first;
@ -481,14 +514,6 @@ export class Batch {
maybe_dirty_effects.clear();
}
unset_batch_values() {
const prev = batch_values;
batch_values = null;
return () => {
batch_values = prev;
};
}
/** @param {(batch: Batch) => void} fn */
oncommit(fn) {
this.#commit_callbacks.add(fn);

@ -40,7 +40,14 @@ import { get_error } from '../../shared/dev.js';
import { async_mode_flag, tracing_mode_flag } from '../../flags/index.js';
import { component_context } from '../context.js';
import { UNINITIALIZED } from '../../../constants.js';
import { batch_values, current_batch } from './batch.js';
import {
batch_values,
current_batch,
has_batch_value_differences,
ignore_batch_values,
set_has_batch_value_differences
} from './batch.js';
import { set_ignore_batch_values } from './batch.js';
import { increment_pending, unset_context } from './async.js';
import { deferred, includes, noop } from '../../shared/utils.js';
import { set_signal_status, update_derived_status } from './status.js';
@ -151,14 +158,15 @@ export function async_derived(fn, label, location) {
// TODO async-nested-derived test shows that we can get here without a current_batch, figure out
// how to handle this case (either throw an error signaling the user they do something wrong or
// turn this into a one-time async derived run). In the meantime defensively call the function.
var restore = batch?.unset_batch_values();
var previous_ignore_batch_values = ignore_batch_values;
set_ignore_batch_values(true);
try {
// 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).finally(unset_context);
} finally {
restore?.();
set_ignore_batch_values(previous_ignore_batch_values);
}
} catch (error) {
d.reject(error);
@ -408,7 +416,31 @@ export function execute_derived(derived) {
*/
export function update_derived(derived) {
var old_value = derived.v;
var value = execute_derived(derived);
// We run the derived first to get the value in the context of the curren batch (if any).
// If batch_values shows that there could be differences in the result of the computation,
// we rerun it again ignoring the batch values. The former value is store in batch_values
// and the latter on derived.v
var value;
var scoped_value;
var previous_ignore_batch_values = ignore_batch_values;
var previous_has_batch_value_differences = has_batch_value_differences;
set_has_batch_value_differences(false);
set_ignore_batch_values(false);
try {
value = execute_derived(derived);
scoped_value = value;
if (has_batch_value_differences || previous_ignore_batch_values) {
set_ignore_batch_values(true);
value = execute_derived(derived);
}
} finally {
set_ignore_batch_values(previous_ignore_batch_values);
set_has_batch_value_differences(previous_has_batch_value_differences);
}
if (!derived.equals(value)) {
derived.wv = increment_write_version();
@ -441,7 +473,7 @@ export function update_derived(derived) {
// only cache the value if we're in a tracking context, otherwise we won't
// clear the cache in `mark_reactions` when dependencies are updated
if (effect_tracking() || current_batch?.is_fork) {
batch_values.set(derived, value);
batch_values.set(derived, scoped_value);
}
} else {
update_derived_status(derived);

@ -27,7 +27,6 @@ import {
} from './constants.js';
import { old_values } from './reactivity/sources.js';
import {
destroy_derived_effects,
execute_derived,
freeze_derived_effects,
recent_async_deriveds,
@ -51,6 +50,9 @@ import {
batch_values,
current_batch,
flushSync,
has_batch_value_differences,
ignore_batch_values,
set_has_batch_value_differences,
schedule_effect
} from './reactivity/batch.js';
import { handle_error } from './error-handling.js';
@ -667,8 +669,12 @@ export function get(signal) {
}
}
if (batch_values?.has(signal)) {
return batch_values.get(signal);
if (!ignore_batch_values && batch_values?.has(signal)) {
var value = batch_values.get(signal);
if (has_batch_value_differences === false && value !== signal.v) {
set_has_batch_value_differences(true);
}
return value;
}
if ((signal.f & ERROR_VALUE) !== 0) {

Loading…
Cancel
Save