fix: flush effects scheduled during boundary's pending phase

[Rich-Harris]

Alternative to #16721, partial alternative to #16709. Closes #16691, closes #16627, closes #16582 and #16651 as well.

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

• 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
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.
• If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: e36137d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

Playground

pnpm add https://pkg.pr.new/svelte@16738 

[Ocean-OS]

Should we add a test for $effect or is the attachment test sufficient?

[dummdidumm]

One question is whether or not $effects inside async components that are scheduled before any async work should flush, in other words if any effects of a component should flush before the component is settled. Because right now the sync effect runs directly. I feel like this shouldn't happen.

[Rich-Harris]

I agree. I'm not immediately sure how to make it behave that way; when the user effects are scheduled on pop, they're included in the next batch.flush(), which doesn't defer effects to the next flush because as far as it's concerned nothing is pending (since async work inside a pending boundary doesn't affect the batch).

So we either need to avoid pushing effects (and pre/render effects? though I guess those aren't scheduled on creation so we probably can't, which is probably fine) to the flush array during traversal, if they're inside a pending boundary (but not block/async effects, obviously), or otherwise exclude them from the flush. But I'm not totally sure how best to make that happen.

I think the best move is to merge this to solve the immediate bug, then I'll open a PR with this failing test so that we don't lose it:

--- a/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/Child.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/Child.svelte @@ -1,7 +1,11 @@ <script> + $effect(() => { + console.log('before'); + }); + await 1; $effect(() => { - console.log('hello'); + console.log('after'); }); </script> diff --git a/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/_config.js b/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/_config.js index 81548a25e..0908b6a9f 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/_config.js @@ -3,7 +3,8 @@ import { test } from '../../test'; export default test({ async test({ assert, logs }) { + assert.deepEqual(logs, []); await tick(); - assert.deepEqual(logs, ['hello']); + assert.deepEqual(logs, ['before', 'after']); } });

[dummdidumm]

confirmed this also fixes #16582