adjust, add test that fails without remove clearing arrays and sets

boundary-batch-nullpointer-fix
Simon Holthausen 2 weeks ago
parent 626b9fcf2b
commit 3ee6ea47c8

@ -1,5 +0,0 @@
---
'svelte': patch
---
fix: associate batch with boundary

@ -59,18 +59,6 @@ export class Boundary {
/** @type {Boundary | null} */
parent;
/**
* The associated batch to this boundary while the boundary pending; set by the one interacting with the boundary when entering pending state.
* Will be `null` once the boundary is no longer pending.
*
* Needed because `current_batch` isn't guaranteed to exist: E.g. when component A has top level await, then renders component B
* which also has top level await, `current_batch` can be null when a flush from component A happens before
* suspend() in component B is called. We hence save it on the boundary instead.
*
* @type {Batch | null}
*/
#batch = null;
/** @type {TemplateNode} */
#anchor;
@ -200,13 +188,6 @@ export class Boundary {
return !!this.#props.pending;
}
get_batch() {
if (current_batch) {
this.#batch = current_batch;
}
return /** @type {Batch} */ (this.#batch);
}
/**
* @param {() => Effect | null} fn
*/
@ -250,7 +231,6 @@ export class Boundary {
if (this.#pending_count === 0) {
this.pending = false;
this.#batch = null;
if (this.#pending_effect) {
pause_effect(this.#pending_effect, () => {

@ -10,7 +10,6 @@ import {
INERT,
RENDER_EFFECT,
ROOT_EFFECT,
USER_EFFECT,
MAYBE_DIRTY
} from '#client/constants';
import { async_mode_flag } from '../../flags/index.js';
@ -405,6 +404,9 @@ export class Batch {
}
remove() {
// Cleanup to
// - prevent memory leaks which could happen if a batch is tied to a never-ending promise
// - prevent effects from rerunning for outdated-and-now-no-longer-pending batches
this.#callbacks.clear();
this.#maybe_dirty_effects =
this.#dirty_effects =
@ -705,7 +707,7 @@ export function schedule_effect(signal) {
export function suspend() {
var boundary = get_pending_boundary();
var batch = boundary.get_batch();
var batch = /** @type {Batch} */ (current_batch);
var pending = boundary.pending;
boundary.update_pending_count(1);

@ -1,4 +1,5 @@
/** @import { Derived, Effect, Source } from '#client' */
/** @import { Batch } from './batch.js'; */
import { DEV } from 'esm-env';
import {
ERROR_VALUE,
@ -32,7 +33,7 @@ import { tracing_mode_flag } from '../../flags/index.js';
import { Boundary } from '../dom/blocks/boundary.js';
import { component_context } from '../context.js';
import { UNINITIALIZED } from '../../../constants.js';
import { batch_deriveds } from './batch.js';
import { batch_deriveds, current_batch } from './batch.js';
import { unset_context } from './async.js';
/** @type {Effect | null} */
@ -130,7 +131,7 @@ export function async_derived(fn, location) {
prev = promise;
var batch = boundary.get_batch();
var batch = /** @type {Batch} */ (current_batch);
var pending = boundary.pending;
if (should_suspend) {

@ -0,0 +1,36 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, logs, target }) {
assert.htmlEqual(
target.innerHTML,
`
<h1>a</h1>
<button>a</button>
<button>b</button>
<button>c</button>
<p>a</p>
`
);
const [a, b] = target.querySelectorAll('button');
b.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<h1>c</h1>
<button>a</button>
<button>b</button>
<button>c</button>
<p>c</p>
<p>b or c</p>
`
);
assert.deepEqual(logs, ['route a', 'route c']);
}
});

@ -0,0 +1,41 @@
<script lang=ts>
let route = $state('a');
function goto(r) {
return Promise.resolve().then(async () => {
route = r;
await Promise.resolve();
});
}
$effect(() => {
console.log('route ' + route);
});
</script>
<h1>{route}</h1>
<button onclick={() => route = 'a'}>a</button>
<button onclick={() => route = 'b'}>b</button>
<button onclick={() => route = 'c'}>c</button>
<svelte:boundary>
{#if route === 'a'}
<p>a</p>
{/if}
{#if route === 'b'}
{await goto('c')}
{/if}
{#if route === 'c'}
<p>c</p>
{/if}
{#if route === 'b' || route === 'c'}
<p>b or c</p>
{/if}
{#snippet pending()}
<p>pending...</p>
{/snippet}
</svelte:boundary>
Loading…
Cancel
Save