Skip to content

Commit

Permalink
fix: ensure derived is detected as dirty correctly
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 f2f71ae commit fc43cff
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 @@ -200,11 +200,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 @@ -230,6 +225,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 fc43cff

Please sign in to comment.