fix: create batch.eager for forks with eager work#18362
Merged
Conversation
Replaces the ad-hoc `STATE_EAGER_EFFECT` flag which makes eager effects over-execute with a mechanism that is two-fold: 1. eager versions are sorted by batch. If a batch belongs to a fork, that way it's made sure it doesn't "escape" that fork 2. batches now have an `eager` field, which is set for forks with eager work. It's flushed on commit just before the main commit. This way the eager work happens separately and eagerly just before the main fork commits, which itself may not be ready yet. Additionally in `sources.js` we make sure `active_batch` is set properly when flushing eager effects.
|
Contributor
|
…er`) is not discarded, causing memory leaks and potentially corrupting the batch linked list.
This commit fixes the issue reported at packages/svelte/src/internal/client/reactivity/batch.js:1464
## Bug Explanation
The bug occurs in the `fork()` function's `discard` method in `batch.js`.
When using `$state.eager()` within a fork, the `eager_flush()` function creates a new Batch for the eager work:
```javascript
function eager_flush(batch) {
flushSync(() => {
const eager_batch = (current_batch = batch?.eager ?? new Batch());
if (batch?.is_fork) {
eager_batch.is_fork = true; // Set to true, meaning #is_deferred() returns true
batch.eager = eager_batch; // Stored on the fork's batch
}
// ...
});
}
```
This eager batch:
1. Is linked into the global batch linked list via `new Batch()`
2. Has `is_fork = true` set, which causes `#is_deferred()` to return true
3. Will never commit or unlink itself because `#is_deferred()` prevents progression
In the fork's `commit` method, this is properly handled:
```javascript
if (batch.eager) {
const eager = batch.eager;
eager.is_fork = false; // Clear the flag
// ... flush the batch ...
eager.flush();
batch.eager = null; // Clear the reference
}
```
But in the `discard` method (before the fix), only `batch.discard()` was called:
```javascript
discard: () => {
// ...
if (!committed && batch.linked) {
batch.discard(); // Only the main batch is discarded!
}
}
```
This leaves the eager batch:
- Still linked in the batch list
- With `is_fork = true` (can never complete)
- Never cleaned up (memory leak)
- Potentially corrupting future batch operations
## Fix Explanation
The fix adds `batch.eager?.discard()` before `batch.discard()` in the fork's discard method:
```javascript
if (!committed && batch.linked) {
batch.eager?.discard(); // Also discard the eager batch if it exists
batch.discard();
}
```
This ensures that when a fork is discarded, any associated eager batch is also properly discarded and unlinked from the batch list, preventing memory leaks and list corruption.
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: Rich-Harris <hello@rich-harris.dev>
Rich-Harris
reviewed
Jun 2, 2026
Co-authored-by: Rich Harris <hello@rich-harris.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the ad-hoc
STATE_EAGER_EFFECTflag which makes eager effects over-execute with a mechanism that is two-fold:eagerfield, which is set for forks with eager work. It's flushed on commit just before the main commit. This way the eager work happens separately and eagerly just before the main fork commits, which itself may not be ready yet.Additionally in
sources.jswe make sureactive_batchis set properly when flushing eager effects.