Skip to content

fix: exclude derived writes from effect abort and rescheduling #16477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/thick-mice-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: exclude derived writes from effect abort and rescheduling
9 changes: 8 additions & 1 deletion packages/svelte/src/internal/client/reactivity/batch.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { invoke_error_boundary } from '../error-handling.js';
import { old_values } from './sources.js';
import { unlink_effect } from './effects.js';
import { unset_context } from './async.js';
import { get_derived_writes, reset_derived_writes } from './deriveds.js';

/** @type {Set<Batch>} */
const batches = new Set();
Expand Down Expand Up @@ -547,6 +548,12 @@ function flush_queued_effects(effects) {
if ((effect.f & (DESTROYED | INERT)) === 0 && is_dirty(effect)) {
var wv = write_version;

// updating a derived for also increase the write version but that doesn't mean
// state was written to in the user effect...so we reset the derived writes
// before running the effect so that we can subtract the amount of derived writes
// from the write version when we detect if state was written to in the user effect
reset_derived_writes();

update_effect(effect);

// Effects with no dependencies or teardown do not get added to the effect tree.
Expand All @@ -568,7 +575,7 @@ function flush_queued_effects(effects) {

// if state is written in a user effect, abort and re-schedule, lest we run
// effects that should be removed as a result of the state change
if (write_version > wv && (effect.f & USER_EFFECT) !== 0) {
if (write_version - get_derived_writes() > wv && (effect.f & USER_EFFECT) !== 0) {
break;
}
}
Expand Down
15 changes: 15 additions & 0 deletions packages/svelte/src/internal/client/reactivity/deriveds.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,20 @@ export function execute_derived(derived) {
return value;
}

// in process_effects if state is written to in a user effect we reschedule the rest of the
// tree. However if a derived is updated in an effect we also increase the write version
// so we need to keep track of how many deriveds were written to in the effect so
// that we can subtract that from the write version before rescheduling unnnecessarily
let derived_writes = 0;

export function get_derived_writes() {
return derived_writes;
}
Comment on lines +332 to +334
Copy link
Member

Choose a reason for hiding this comment

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

FWIW we don't need to expose function wrappers to get values from different modules, only to set them. we can just export let derived_writes

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm always so skeptical of live exports...like under my skin I know they works but I just can't trust myself enough to do it 😂


export function reset_derived_writes() {
derived_writes = 0;
}

/**
* @param {Derived} derived
* @returns {void}
Expand All @@ -333,6 +347,7 @@ export function update_derived(derived) {
if (!derived.equals(value)) {
derived.v = value;
derived.wv = increment_write_version();
derived_writes++;
}

// don't mark derived clean if we're reading it inside a
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
const der = $derived(false);

$effect(() => {
der
});
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { test } from '../../test';

export default test({
async test() {}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import Component from './Component.svelte'
const arr = Array.from({length: 10001});
</script>

{#each arr}
<Component />
{/each}
Loading