fix: unset_context when not already in an async derived#18295
Conversation
|
|
dummdidumm
left a comment
There was a problem hiding this comment.
approving but see the comment for small nit
| if (batch === current_batch) unset_context(); | ||
| }); | ||
| } | ||
| if (unset) queue_micro_task(unset_context); |
There was a problem hiding this comment.
Is the if (batch === current_batch) to check that it's still about our batch wrong or why did you remove it? Also I think the comment helps understand why we need this.
There was a problem hiding this comment.
Not wrong, just probably unnecessary — I can't make batch !== current_batch. I guess it's relatively harmless to include the check though
|
On second thoughts, I think we should mostly revert this. Over on We long ago decided that we shouldn't be restoring the batch context after an async function sum() {
return await a + b;
}
let total = $derived(await sum());We made that decision so that code couldn't start behaving inconsistently depending on whether the compiler could see it. But if that applies to dependency tracking, then it should apply equally to whether or not we're allowed to create effects. So the fact that we call |
This is the fix in #18289 (comment), but saves us creating a bunch of microtasks