Skip to content

Commit

Permalink
fix: ensure derived is detected as dirty correctly (#11496)
Browse files Browse the repository at this point in the history
Deriveds where under certain conditions not detected as dirty correctly. The reason is that a transitive check_dirtiness call could update the flag of a derived, even if the condition doesn't ulimately result to true. That's why the check for "is now dirty" needs to be moved out of the inner if block.
Fixes #11481

This may also fix a yet undetected overfiring bug in the "is unowned" case because the previous inner "is now dirty?" check didn't take unowned into account.
  • Loading branch information
dummdidumm committed May 7, 2024
1 parent d86b052 commit 6bd6f09
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/shiny-melons-love.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: ensure derived is detected as dirty correctly
8 changes: 3 additions & 5 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,6 @@ export function check_dirtiness(reaction) {

if (!is_dirty && check_dirtiness(/** @type {import('#client').Derived} */ (dependency))) {
update_derived(/** @type {import('#client').Derived} **/ (dependency), true);

// `signal` might now be dirty, as a result of calling `update_derived`
if ((reaction.f & DIRTY) !== 0) {
return true;
}
}

if (is_unowned) {
Expand All @@ -227,6 +222,9 @@ export function check_dirtiness(reaction) {
reactions.push(reaction);
}
}
} else if ((reaction.f & DIRTY) !== 0) {
// `signal` might now be dirty, as a result of calling `check_dirtiness` and/or `update_derived`
return true;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { test } from '../../test';

export default test({
html: `<button>00</button>`,

async test({ assert, target }) {
const btn = target.querySelector('button');
await btn?.click();

assert.htmlEqual(target.innerHTML, `<button>01</button>`);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
let shouldShow01 = $state(false);
let der1 = $derived(shouldShow01)
// der2 must depend on der1 and its output shouldn't change
let der2 = $derived(typeof der1 === "string");
let der3 = $derived(der2 ? "1" : "0");
// der3 must be read before der1
let der4 = $derived(der3 + (der1 ? "1" : "0"));
</script>

<button onclick={() => (shouldShow01 = true)}>{der4}</button>

0 comments on commit 6bd6f09

Please sign in to comment.