Skip to content

Conversation

@dummdidumm
Copy link
Member

When deferring effects we didn't unmark the deriveds that lead to those effects. This means that they might not be reached in subsequent runs of mark_reactions.

Fixes #17118 (comment)

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

When deferring effects we didn't unmark the deriveds that lead to those effects. This means that they might not be reached in subsequent runs of `mark_reactions`.

Fixes #17118 (comment)
@changeset-bot
Copy link

changeset-bot bot commented Nov 12, 2025

🦋 Changeset detected

Latest commit: 8f7e8b9

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@17147

@paoloricciuti
Copy link
Member

Btw to fully solve this in SvelteKit I think we also need to discard the last fork when a new one come because I think right now we don't do it.

@dummdidumm
Copy link
Member Author

Why though? It should be possible to have parallel forks. They could be independent and it would be very confusing to have a fork be discarded when another part of the app you may not have control over starts a fork

@paoloricciuti
Copy link
Member

Mmm I'm specifically talking about preloading tho... shouldn't we discard older fork when you move your mouse on another link?

@dummdidumm
Copy link
Member Author

Looking at SvelteKit's code it should alread do that (discard_load_cache)

@paoloricciuti
Copy link
Member

It only do that inside goto, invalidateAll or if it errors...we also do that in preload before creating the new load_cache (this also causes await reactivity loss warnings). I might be wrong tho.

@dummdidumm
Copy link
Member Author

Moving your mouse on another link should call preload and therefore discard the old cache, no?

@paoloricciuti
Copy link
Member

Yeah but right now preload doesn't discard the cache unless it's an error...that's the issue.

image

@paoloricciuti
Copy link
Member

Lul just realized a missed a "should" before the last sentence 😅

@PatrickG
Copy link
Member

PatrickG commented Nov 13, 2025

@paoloricciuti I think Rich already implemented it sveltejs/kit#14865 it is just not released yet.
But TBH I don't really understand why we need that. Wouldn't it be enough to discard all other forks when the user finally clicks a link.

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.

Experimental Async with preload and conditional bind:this kills reactivity

4 participants