Skip to content

Commit d3a69c5

Browse files
committed
fix: access last safe value of prop on unmount
1 parent be82332 commit d3a69c5

File tree

9 files changed

+185
-6
lines changed

9 files changed

+185
-6
lines changed

.changeset/mean-dryers-fix.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: access last safe value of prop on unmount

packages/svelte/src/internal/client/constants.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,6 @@ export const STATE_SYMBOL = Symbol('$state');
2525
export const STATE_SYMBOL_METADATA = Symbol('$state metadata');
2626
export const LEGACY_PROPS = Symbol('legacy props');
2727
export const LOADING_ATTR_SYMBOL = Symbol('');
28+
29+
export const CTX_CONTAINS_TEARDOWN = 1;
30+
export const CTX_DESTROYED = 2;

packages/svelte/src/internal/client/context.js

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ import {
1111
set_active_reaction,
1212
untrack
1313
} from './runtime.js';
14-
import { effect } from './reactivity/effects.js';
14+
import { effect, teardown } from './reactivity/effects.js';
1515
import { legacy_mode_flag } from '../flags/index.js';
16+
import { CTX_CONTAINS_TEARDOWN, CTX_DESTROYED } from './constants.js';
1617

1718
/** @type {ComponentContext | null} */
1819
export let component_context = null;
@@ -112,15 +113,17 @@ export function getAllContexts() {
112113
* @returns {void}
113114
*/
114115
export function push(props, runes = false, fn) {
115-
component_context = {
116+
var ctx = (component_context = {
116117
p: component_context,
117118
c: null,
118119
e: null,
120+
f: 0,
119121
m: false,
120122
s: props,
121123
x: null,
122-
l: null
123-
};
124+
l: null,
125+
tp: props
126+
});
124127

125128
if (legacy_mode_flag && !runes) {
126129
component_context.l = {
@@ -131,6 +134,24 @@ export function push(props, runes = false, fn) {
131134
};
132135
}
133136

137+
teardown(() => {
138+
if (ctx.f !== CTX_CONTAINS_TEARDOWN) {
139+
return;
140+
}
141+
// Mark the context as destroyed, so any derived props can use
142+
// the latest known value before teardown
143+
ctx.f = CTX_DESTROYED;
144+
145+
var teardown_props = ctx.tp;
146+
// Apply the latest known props before teardown over existing props
147+
for (var key in teardown_props) {
148+
Object.defineProperty(props, key, {
149+
value: teardown_props[key],
150+
configurable: true
151+
});
152+
}
153+
});
154+
134155
if (DEV) {
135156
// component function
136157
component_context.function = fn;
@@ -171,6 +192,12 @@ export function pop(component) {
171192
dev_current_component_function = context_stack_item.p?.function ?? null;
172193
}
173194
context_stack_item.m = true;
195+
196+
effect(() => {
197+
if (context_stack_item.f === CTX_CONTAINS_TEARDOWN) {
198+
context_stack_item.tp = { ...context_stack_item.s };
199+
}
200+
});
174201
}
175202
// Micro-optimization: Don't set .a above to the empty object
176203
// so it can be garbage-collected when the return here is unused

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import { safe_equals } from './equality.js';
2323
import * as e from '../errors.js';
2424
import {
2525
BRANCH_EFFECT,
26+
CTX_DESTROYED,
27+
DESTROYED,
2628
LEGACY_DERIVED_PROP,
2729
LEGACY_PROPS,
2830
ROOT_EFFECT,
@@ -31,6 +33,7 @@ import {
3133
import { proxy } from '../proxy.js';
3234
import { capture_store_binding } from './store.js';
3335
import { legacy_mode_flag } from '../../flags/index.js';
36+
import { component_context } from '../context.js';
3437

3538
/**
3639
* @param {((value?: number) => number)} fn
@@ -369,6 +372,12 @@ export function prop(props, key, flags, fallback) {
369372
// source is written to from various places to persist this value.
370373
var inner_current_value = mutable_source(prop_value);
371374
var current_value = derived(() => {
375+
var ctx = component_context;
376+
377+
if (ctx !== null && ctx.f === CTX_DESTROYED) {
378+
return get(inner_current_value);
379+
}
380+
372381
var parent_value = getter();
373382
var child_value = get(inner_current_value);
374383

@@ -413,6 +422,7 @@ export function prop(props, key, flags, fallback) {
413422

414423
return value;
415424
}
425+
416426
return get(current_value);
417427
};
418428
}

packages/svelte/src/internal/client/runtime.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ import {
2222
ROOT_EFFECT,
2323
LEGACY_DERIVED_PROP,
2424
DISCONNECTED,
25-
BOUNDARY_EFFECT
25+
BOUNDARY_EFFECT,
26+
CTX_CONTAINS_TEARDOWN
2627
} from './constants.js';
2728
import { flush_tasks } from './dom/task.js';
2829
import { internal_set } from './reactivity/sources.js';
@@ -566,7 +567,14 @@ export function update_effect(effect) {
566567

567568
execute_effect_teardown(effect);
568569
var teardown = update_reaction(effect);
569-
effect.teardown = typeof teardown === 'function' ? teardown : null;
570+
if (typeof teardown === 'function') {
571+
if (effect.ctx !== null && effect.ctx.f === 0) {
572+
effect.ctx.f = CTX_CONTAINS_TEARDOWN;
573+
}
574+
effect.teardown = teardown;
575+
} else {
576+
effect.teardown = null;
577+
}
570578
effect.wv = write_version;
571579

572580
var deps = effect.deps;

packages/svelte/src/internal/client/types.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ export type ComponentContext = {
2020
effect: null | Effect;
2121
reaction: null | Reaction;
2222
}>;
23+
/** ctx flags */
24+
f: number;
2325
/** mounted */
2426
m: boolean;
2527
/**
@@ -57,6 +59,8 @@ export type ComponentContext = {
5759
* dev mode only: the component function
5860
*/
5961
function?: any;
62+
/** teardown props */
63+
tp: Record<string, unknown>;
6064
};
6165

6266
export type ComponentContextLegacy = ComponentContext & {
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
let { checked = $bindable(), count = $bindable() } = $props();
3+
$effect(() => ()=>{
4+
console.log(count, checked);
5+
});
6+
</script>
7+
8+
<p>{count}</p>
9+
10+
<button onclick={()=> count-- }></button>
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { ok, test } from '../../test';
2+
import { flushSync } from 'svelte';
3+
4+
export default test({
5+
async test({ assert, target, logs }) {
6+
const [btn1, btn2, btn3] = target.querySelectorAll('button');
7+
let ps = [...target.querySelectorAll('p')];
8+
9+
for (const p of ps) {
10+
assert.equal(p.innerHTML, '0');
11+
}
12+
13+
flushSync(() => {
14+
btn1.click();
15+
});
16+
17+
// prop update normally if we are not unmounting
18+
for (const p of ps) {
19+
assert.equal(p.innerHTML, '1');
20+
}
21+
22+
flushSync(() => {
23+
btn3.click();
24+
});
25+
26+
// binding still works and update the value correctly
27+
for (const p of ps) {
28+
assert.equal(p.innerHTML, '0');
29+
}
30+
31+
flushSync(() => {
32+
btn1.click();
33+
});
34+
35+
flushSync(() => {
36+
btn1.click();
37+
});
38+
39+
console.warn(logs);
40+
41+
// the five components guarded by `count < 2` unmount and log
42+
assert.deepEqual(logs, [1, true, 1, true, 1, true, 1, true, 1, true]);
43+
44+
flushSync(() => {
45+
btn2.click();
46+
});
47+
48+
// the three components guarded by `show` unmount and log
49+
assert.deepEqual(logs, [
50+
1,
51+
true,
52+
1,
53+
true,
54+
1,
55+
true,
56+
1,
57+
true,
58+
1,
59+
true,
60+
2,
61+
true,
62+
2,
63+
true,
64+
2,
65+
false
66+
]);
67+
}
68+
});
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<script>
2+
import Component from "./Component.svelte";
3+
4+
let show = $state(true);
5+
let count = $state(0);
6+
let spread = $derived({ checked: show, count });
7+
let Dynamic = $derived(count < 2 ? Component : undefined);
8+
let Dynamic2 = $derived(show ? Component : undefined);
9+
</script>
10+
11+
<button onclick={()=> count++ }></button>
12+
<button onclick={()=> show = !show }></button>
13+
14+
<!-- count with bind -->
15+
{#if count < 2}
16+
<Component bind:count bind:checked={show} />
17+
{/if}
18+
19+
<!-- spread syntax -->
20+
{#if count < 2}
21+
<Component {...spread} />
22+
{/if}
23+
24+
<!-- normal prop -->
25+
{#if count < 2}
26+
<Component {count} checked={show} />
27+
{/if}
28+
29+
<!-- prop only accessed in destroy -->
30+
{#if show}
31+
<Component {count} checked={show} />
32+
{/if}
33+
34+
<!-- dynamic component -->
35+
<Dynamic {count} checked={show} />
36+
37+
<!-- dynamic component spread -->
38+
<Dynamic {...spread} />
39+
40+
<!-- dynamic component with prop only accessed on destroy -->
41+
<Dynamic2 {count} checked={show} />
42+
43+
<!-- dynamic component with prop only accessed on destroy spread -->
44+
<Dynamic2 {...spread} />

0 commit comments

Comments
 (0)