Skip to content

Commit 3ee6ea4

Browse files
committed
adjust, add test that fails without remove clearing arrays and sets
1 parent 626b9fc commit 3ee6ea4

File tree

6 files changed

+84
-29
lines changed

6 files changed

+84
-29
lines changed

.changeset/spicy-ears-join.md

Lines changed: 0 additions & 5 deletions
This file was deleted.

packages/svelte/src/internal/client/dom/blocks/boundary.js

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -59,18 +59,6 @@ export class Boundary {
5959
/** @type {Boundary | null} */
6060
parent;
6161

62-
/**
63-
* The associated batch to this boundary while the boundary pending; set by the one interacting with the boundary when entering pending state.
64-
* Will be `null` once the boundary is no longer pending.
65-
*
66-
* Needed because `current_batch` isn't guaranteed to exist: E.g. when component A has top level await, then renders component B
67-
* which also has top level await, `current_batch` can be null when a flush from component A happens before
68-
* suspend() in component B is called. We hence save it on the boundary instead.
69-
*
70-
* @type {Batch | null}
71-
*/
72-
#batch = null;
73-
7462
/** @type {TemplateNode} */
7563
#anchor;
7664

@@ -200,13 +188,6 @@ export class Boundary {
200188
return !!this.#props.pending;
201189
}
202190

203-
get_batch() {
204-
if (current_batch) {
205-
this.#batch = current_batch;
206-
}
207-
return /** @type {Batch} */ (this.#batch);
208-
}
209-
210191
/**
211192
* @param {() => Effect | null} fn
212193
*/
@@ -250,7 +231,6 @@ export class Boundary {
250231

251232
if (this.#pending_count === 0) {
252233
this.pending = false;
253-
this.#batch = null;
254234

255235
if (this.#pending_effect) {
256236
pause_effect(this.#pending_effect, () => {

packages/svelte/src/internal/client/reactivity/batch.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
INERT,
1111
RENDER_EFFECT,
1212
ROOT_EFFECT,
13-
USER_EFFECT,
1413
MAYBE_DIRTY
1514
} from '#client/constants';
1615
import { async_mode_flag } from '../../flags/index.js';
@@ -405,6 +404,9 @@ export class Batch {
405404
}
406405

407406
remove() {
407+
// Cleanup to
408+
// - prevent memory leaks which could happen if a batch is tied to a never-ending promise
409+
// - prevent effects from rerunning for outdated-and-now-no-longer-pending batches
408410
this.#callbacks.clear();
409411
this.#maybe_dirty_effects =
410412
this.#dirty_effects =
@@ -705,7 +707,7 @@ export function schedule_effect(signal) {
705707

706708
export function suspend() {
707709
var boundary = get_pending_boundary();
708-
var batch = boundary.get_batch();
710+
var batch = /** @type {Batch} */ (current_batch);
709711
var pending = boundary.pending;
710712

711713
boundary.update_pending_count(1);

packages/svelte/src/internal/client/reactivity/deriveds.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/** @import { Derived, Effect, Source } from '#client' */
2+
/** @import { Batch } from './batch.js'; */
23
import { DEV } from 'esm-env';
34
import {
45
ERROR_VALUE,
@@ -32,7 +33,7 @@ import { tracing_mode_flag } from '../../flags/index.js';
3233
import { Boundary } from '../dom/blocks/boundary.js';
3334
import { component_context } from '../context.js';
3435
import { UNINITIALIZED } from '../../../constants.js';
35-
import { batch_deriveds } from './batch.js';
36+
import { batch_deriveds, current_batch } from './batch.js';
3637
import { unset_context } from './async.js';
3738

3839
/** @type {Effect | null} */
@@ -130,7 +131,7 @@ export function async_derived(fn, location) {
130131

131132
prev = promise;
132133

133-
var batch = boundary.get_batch();
134+
var batch = /** @type {Batch} */ (current_batch);
134135
var pending = boundary.pending;
135136

136137
if (should_suspend) {
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { tick } from 'svelte';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
async test({ assert, logs, target }) {
6+
assert.htmlEqual(
7+
target.innerHTML,
8+
`
9+
<h1>a</h1>
10+
<button>a</button>
11+
<button>b</button>
12+
<button>c</button>
13+
<p>a</p>
14+
`
15+
);
16+
17+
const [a, b] = target.querySelectorAll('button');
18+
19+
b.click();
20+
await tick();
21+
22+
assert.htmlEqual(
23+
target.innerHTML,
24+
`
25+
<h1>c</h1>
26+
<button>a</button>
27+
<button>b</button>
28+
<button>c</button>
29+
<p>c</p>
30+
<p>b or c</p>
31+
`
32+
);
33+
34+
assert.deepEqual(logs, ['route a', 'route c']);
35+
}
36+
});
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<script lang=ts>
2+
let route = $state('a');
3+
4+
function goto(r) {
5+
return Promise.resolve().then(async () => {
6+
route = r;
7+
await Promise.resolve();
8+
});
9+
}
10+
11+
$effect(() => {
12+
console.log('route ' + route);
13+
});
14+
</script>
15+
16+
<h1>{route}</h1>
17+
<button onclick={() => route = 'a'}>a</button>
18+
<button onclick={() => route = 'b'}>b</button>
19+
<button onclick={() => route = 'c'}>c</button>
20+
21+
<svelte:boundary>
22+
{#if route === 'a'}
23+
<p>a</p>
24+
{/if}
25+
26+
{#if route === 'b'}
27+
{await goto('c')}
28+
{/if}
29+
30+
{#if route === 'c'}
31+
<p>c</p>
32+
{/if}
33+
34+
{#if route === 'b' || route === 'c'}
35+
<p>b or c</p>
36+
{/if}
37+
38+
{#snippet pending()}
39+
<p>pending...</p>
40+
{/snippet}
41+
</svelte:boundary>

0 commit comments

Comments
 (0)