fix: recover from errors that leave a corrupted effect tree (#17888)

https://github.com/sveltejs/svelte/pull/17680#issuecomment-3888440736.
Errors that occur during traversal (not inside a template effect etc)
can leave dirty effects inside the effect tree, but with clean parents.
This means that

a) subsequent changes to their dependencies won't schedule them to
re-run
b) subsequent batch flushes won't 'reach' them unless a sibling effect
happens to be made dirty

The easiest way to fix this is to just repair the tree if traversal
fails. If you had a truly ginormous tree this could conceivably take a
noticeable amount of time, but that's probably better than the app just
being broken.

Note that this doesn't apply to errors that occur inside an error
boundary, because in that case the offending subtree gets destroyed.
This is just for errors that bubble all the way to the root.

Closes #17680, closes #17679.
pull/17914/head
Rich Harris 7 months ago committed by GitHub
parent e4e089310d
commit 667896a753
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: recover from errors that leave a corrupted effect tree

@ -225,7 +225,12 @@ export class Batch {
var updates = (legacy_updates = []);
for (const root of roots) {
this.#traverse(root, effects, render_effects);
try {
this.#traverse(root, effects, render_effects);
} catch (e) {
reset_all(root);
throw e;
}
}
// any writes should take effect in a subsequent batch
@ -959,6 +964,20 @@ function reset_branch(effect, tracked) {
}
}
/**
* Mark an entire effect tree clean following an error
* @param {Effect} effect
*/
function reset_all(effect) {
set_signal_status(effect, CLEAN);
var e = effect.first;
while (e !== null) {
reset_all(e);
e = e.next;
}
}
/**
* Creates a 'fork', in which state changes are evaluated but not applied to the DOM.
* This is useful for speculatively loading data (for example) when you suspect that

@ -0,0 +1,32 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target, compileOptions }) {
const [toggle, increment] = target.querySelectorAll('button');
flushSync(() => increment.click());
assert.htmlEqual(
target.innerHTML,
`
<button>toggle</button>
<button>count: 1</button>
<p>show: false</p>
`
);
assert.throws(() => {
flushSync(() => toggle.click());
}, /NonExistent is not defined/);
flushSync(() => increment.click());
assert.htmlEqual(
target.innerHTML,
`
<button>toggle</button>
<button>count: 2</button>
<p>show: ${compileOptions.experimental?.async ? 'false' : 'true'}</p>
`
);
}
});

@ -0,0 +1,13 @@
<script>
let show = $state(false);
let count = $state(0);
</script>
<button onclick={() => show = !show}>toggle</button>
<button onclick={() => count += 1}>count: {count}</button>
<p>show: {show}</p>
{#if show}
<NonExistent />
{/if}
Loading…
Cancel
Save