fix: disallow effect creation after `await` (#18299)

We incorrectly restore effect context after an`$derived(await ...)` if
it occurs inside an async function, but only for those, no other `await`
expressions. This is buggy and wrong. We only want to restore context
_inside_ an async derived expression (including template expressions)`
so that `await a + b` works correctly.

It's possible that this will be a breaking change (albeit not
semver-violating, because `experimental`) for some people if they are
creating effects after an `await` (other than at the top level of a
component). This is regrettable, but it should never have worked in the
first place.
pull/18311/head
Rich Harris 2 months ago committed by GitHub
parent d4be4e2973
commit 0da9f9e2ab
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -88,6 +88,10 @@ Effect cannot be created inside a `$derived` value that was not itself created i
`%rune%` can only be used inside an effect (e.g. during component initialisation)
```
Effects can only be created while a parent effect is running. This means that they cannot, for example, be created inside an event handler or after an `await` expression (unless the `await` occurs directly inside a component's `<script>` tag, and not inside an async function).
In very rare cases, it is appropriate to use [`$effect.root`]($effect#$effect.root) so that you can create effects outside the normal component lifecycle.
### effect_pending_outside_reaction
```

@ -60,6 +60,10 @@ The key expression in a keyed each block must return the same value when called
> `%rune%` can only be used inside an effect (e.g. during component initialisation)
Effects can only be created while a parent effect is running. This means that they cannot, for example, be created inside an event handler or after an `await` expression (unless the `await` occurs directly inside a component's `<script>` tag, and not inside an async function).
In very rare cases, it is appropriate to use [`$effect.root`]($effect#$effect.root) so that you can create effects outside the normal component lifecycle.
## effect_pending_outside_reaction
> `$effect.pending()` can only be called inside an effect or derived

@ -115,8 +115,7 @@ function is_last_evaluated_expression(path, node) {
break;
case 'MemberExpression':
if (parent.computed && node === parent.object) return false;
break;
return false;
case 'ObjectExpression':
if (node !== parent.properties.at(-1)) return false;

@ -194,11 +194,6 @@ export function VariableDeclaration(node, context) {
/** @type {CallExpression} */ (init)
);
// for now, only wrap async derived in $.save if it's not
// a top-level instance derived. TODO in future maybe we
// can dewaterfall all of them?
const should_save = context.state.is_instance && context.state.scope.function_depth > 1;
if (declarator.id.type === 'Identifier') {
let expression = /** @type {Expression} */ (context.visit(value));
@ -213,9 +208,7 @@ export function VariableDeclaration(node, context) {
location ? b.literal(location) : undefined
);
call = should_save ? save(call, true) : b.await(call);
declarations.push(b.declarator(declarator.id, call));
declarations.push(b.declarator(declarator.id, b.await(call)));
} else {
if (rune === '$derived') expression = b.thunk(expression);
@ -251,7 +244,7 @@ export function VariableDeclaration(node, context) {
location ? b.literal(location) : undefined
);
call = should_save ? save(call, true) : b.await(call);
call = b.await(call);
}
declarations.push(b.declarator(id, call));

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

@ -25,7 +25,6 @@ import {
set_reactivity_loss_tracker
} from './deriveds.js';
import { aborted } from './effects.js';
import { queue_micro_task } from '../dom/task.js';
/**
* @param {Blocker[]} blockers
@ -149,25 +148,13 @@ export function capture() {
* `await a + b` becomes `(await $.save(a))() + b`
* @template T
* @param {Promise<T>} promise
* @param {boolean} unset
* @returns {Promise<() => T>}
*/
export async function save(promise, unset) {
var batch = current_batch;
export async function save(promise) {
var restore = capture();
var value = await promise;
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();
return value;
};

@ -6,14 +6,13 @@
async function fn() {
let count = $state(1);
increment = () => { count++; };
$effect.pre(() => console.log(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,12 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, logs }) {
await tick();
assert.deepEqual(logs, [
`effect_orphan\n\`$effect\` can only be used inside an effect (e.g. during component initialisation)\nhttps://svelte.dev/e/effect_orphan`
]);
}
});

@ -0,0 +1,14 @@
<script>
async function foo() {
await 0;
$effect(() => {
console.log('this effect is orphaned');
});
}
try {
await foo();
} catch (e) {
console.log(e.message)
}
</script>

@ -1,14 +0,0 @@
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]);
}
});

@ -1,15 +0,0 @@
<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