Skip to content

fix: run effects in pending snippets#17719

Merged
Rich-Harris merged 18 commits intomainfrom
boundary-pending-live
Feb 17, 2026
Merged

fix: run effects in pending snippets#17719
Rich-Harris merged 18 commits intomainfrom
boundary-pending-live

Conversation

@Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Feb 16, 2026

Boundaries are buggy: if the pending snippet contains state, changes to that state won't cause updates:

<script>
	let resolvers = [];

	function push(value) {
		const deferred = Promise.withResolvers();
		resolvers.push(() => deferred.resolve(value));
		return deferred.promise;
	}

	function shift() {
		resolvers.shift()?.();
	}

	let count = $state(0);
</script>

<button onclick={() => count += 1}>
	increment
</button>

<button onclick={shift}>
	shift
</button>

<svelte:boundary>
	<p>{await push('resolved')}</p>

	{#snippet pending()}
		<p>{count}</p>
	{/snippet}
</svelte:boundary>

The issue is that the boundary's this.#effect has the BOUNDARY_EFFECT flag, and this.#pending_effect is a child thereof. Instead, this.#main_effect should have the flag. (It turns out this.#failed_effect also needs the flag, because errors that occur in a failed snippet cause the boundary to re-render in its failed state, which I found somewhat confusing to be honest. Probably the right choice though.)

I was able to simplify the code a bit, too.

(Actually now that I think about it do we need this.#effect at all? Will check.)

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
Copy link

changeset-bot bot commented Feb 16, 2026

🦋 Changeset detected

Latest commit: 02a8f39

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
Copy link
Contributor

Playground

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

@Rich-Harris
Copy link
Member Author

gah, just realised this will likely conflict with #17672. wanted to get this PR in ahead of working on #16743, which in my head is blocking some other stuff, but we should probably to #17672 first

@Rich-Harris Rich-Harris marked this pull request as ready for review February 16, 2026 23:08
Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good but this has the consequence that there's now no more state updates in a failed state: playground

This is arguably worse than the short-lived pending snippet.

If fixing this means removing those lines I added the TODO to, then I think it's reasonable to make this very pedantically strictly speaking breaking change which noone will run into in practise and bubble up an error within a failed snippet to the next boundary (and also add a test to codify this)

Nevermind this was a playground bug not firing events

@Rich-Harris Rich-Harris merged commit 6557a0a into main Feb 17, 2026
18 checks passed
@Rich-Harris Rich-Harris deleted the boundary-pending-live branch February 17, 2026 13:12
@github-actions github-actions bot mentioned this pull request Feb 17, 2026
Rich-Harris pushed a commit that referenced this pull request Feb 17, 2026
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## svelte@5.51.3

### Patch Changes

- fix: prevent event delegation logic conflicting between svelte
instances ([#17728](#17728))

- fix: treat CSS attribute selectors as case-insensitive for HTML
enumerated attributes
([#17712](#17712))

- fix: locate Rollup annontaion friendly to JS downgraders
([#17724](#17724))

- fix: run effects in pending snippets
([#17719](#17719))

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants