fix: don't assume boundary exists during increment/decrement (#18289)

Follow-up to #18273 (not merged yet hence no changeset here): We can run
into a null-pointer when wanting to increment/decrement inside an effect
root that is outside a the component tree. Similarly, if not using the
component logic to unset context at the right time we gotta do it
"manually" inside `save`.

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
pull/18302/head
Simon H 4 months ago committed by GitHub
parent c01e598fff
commit b40c359d44
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -213,7 +213,7 @@ export function VariableDeclaration(node, context) {
location ? b.literal(location) : undefined location ? b.literal(location) : undefined
); );
call = should_save ? save(call) : b.await(call); call = should_save ? save(call, true) : b.await(call);
declarations.push(b.declarator(declarator.id, call)); declarations.push(b.declarator(declarator.id, call));
} else { } else {
@ -251,7 +251,7 @@ export function VariableDeclaration(node, context) {
location ? b.literal(location) : undefined location ? b.literal(location) : undefined
); );
call = should_save ? save(call) : b.await(call); call = should_save ? save(call, true) : b.await(call);
} }
declarations.push(b.declarator(id, call)); declarations.push(b.declarator(id, call));

@ -633,7 +633,8 @@ export function has_await_expression(node) {
/** /**
* Turns `await ...` to `(await $.save(...))()` * Turns `await ...` to `(await $.save(...))()`
* @param {ESTree.Expression} expression * @param {ESTree.Expression} expression
* @param {boolean} unset
*/ */
export function save(expression) { export function save(expression, unset = false) {
return b.call(b.await(b.call('$.save', expression))); return b.call(b.await(b.call('$.save', expression, unset && b.true)));
} }

@ -25,6 +25,7 @@ import {
set_reactivity_loss_tracker set_reactivity_loss_tracker
} from './deriveds.js'; } from './deriveds.js';
import { aborted } from './effects.js'; import { aborted } from './effects.js';
import { queue_micro_task } from '../dom/task.js';
/** /**
* @param {Blocker[]} blockers * @param {Blocker[]} blockers
@ -148,13 +149,25 @@ export function capture() {
* `await a + b` becomes `(await $.save(a))() + b` * `await a + b` becomes `(await $.save(a))() + b`
* @template T * @template T
* @param {Promise<T>} promise * @param {Promise<T>} promise
* @param {boolean} unset
* @returns {Promise<() => T>} * @returns {Promise<() => T>}
*/ */
export async function save(promise) { export async function save(promise, unset) {
var batch = current_batch;
var restore = capture(); var restore = capture();
var value = await promise; var value = await promise;
return () => { return () => {
if (unset) {
// If this is happening outside the context of an async derived,
// context will not automatically be unset
queue_micro_task(() => {
if (batch === current_batch) {
unset_context();
}
});
}
restore(); restore();
return value; return value;
}; };
@ -352,15 +365,15 @@ export function wait(blockers) {
*/ */
export function increment_pending() { export function increment_pending() {
var effect = /** @type {Effect} */ (active_effect); var effect = /** @type {Effect} */ (active_effect);
var boundary = /** @type {Boundary} */ (effect.b); var boundary = effect.b; // undefined if called outside the render tree, e.g. a standalone $effect.root
var batch = /** @type {Batch} */ (current_batch); var batch = /** @type {Batch} */ (current_batch);
var blocking = boundary.is_rendered(); var blocking = !!boundary?.is_rendered();
boundary.update_pending_count(1, batch); boundary?.update_pending_count(1, batch);
batch.increment(blocking, effect); batch.increment(blocking, effect);
return () => { return () => {
boundary.update_pending_count(-1, batch); boundary?.update_pending_count(-1, batch);
batch.decrement(blocking, effect); batch.decrement(blocking, effect);
}; };
} }

@ -1,11 +0,0 @@
<script>
setTimeout(() => {
$effect.root(() => {
async function fn() {
const value = $derived(await 1);
return { get value() { return value } };
}
fn().then(r => console.log(r.value));
})
})
</script>

@ -1,9 +1,15 @@
import { tick } from 'svelte';
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
// Test that an async derived inside an $effect.root not connected to the component tree still works // Test that an async derived inside an $effect.root not connected to the component tree still works
async test({ assert, logs }) { async test({ assert, logs }) {
await new Promise((resolve) => setTimeout(resolve, 10)); await new Promise((resolve) => setTimeout(resolve, 10));
assert.deepEqual(logs, [1]); assert.deepEqual(logs, [1, 1]);
const [button] = document.querySelectorAll('button');
button.click();
await tick();
assert.deepEqual(logs, [1, 1, 2]);
} }
}); });

@ -0,0 +1,19 @@
<script>
let increment;
setTimeout(() => {
$effect.root(() => {
async function fn() {
let count = $state(1);
increment = () => { count++; };
const value = $derived(await count);
$effect.pre(() => console.log(value))
return { get value() { return value } };
}
fn().then(r => console.log(r.value));
})
})
</script>
<button onclick={() => increment()}>increment</button>

@ -0,0 +1,14 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, logs }) {
await new Promise((resolve) => setTimeout(resolve, 10));
assert.deepEqual(logs, [1, 1]);
const [button] = document.querySelectorAll('button');
button.click();
await tick();
assert.deepEqual(logs, [1, 1, 2]);
}
});

@ -0,0 +1,15 @@
<script>
let increment;
async function fn() {
let count = $state(1);
increment = () => { count++; };
const value = $derived(await count);
$effect.pre(() => console.log(value))
return { get value() { return value } };
}
fn().then(r => console.log(r.value));
</script>
<button onclick={() => increment()}>increment</button>
Loading…
Cancel
Save