Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f510201

Browse filesBrowse files
dummdidummdummdidumm
authored
fix: better interop of $state with actions/$: statements (#10543)
Ensure update methods of actions and reactive statements work with fine-grained `$state` by deep-reading them. This is necessary because mutations to `$state` objects don't notify listeners to only the object as a whole, it only notifies the listeners of the property that changed. fixes #10460 fixes simeydotme/svelte-range-slider-pips#130
1 parent 4c9d71f commit f510201
Copy full SHA for f510201

File tree

Expand file treeCollapse file tree

12 files changed

+113
-18
lines changed
Filter options
Expand file treeCollapse file tree

12 files changed

+113
-18
lines changed

‎.changeset/light-days-clean.md

Copy file name to clipboard
+5Lines 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: ensure update methods of actions and reactive statements work with fine-grained `$state`

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js

Copy file name to clipboardExpand all lines: packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,10 @@ export const javascript_visitors_legacy = {
167167
const name = binding.node.name;
168168
let serialized = serialize_get_binding(b.id(name), state);
169169

170-
if (name === '$$props' || name === '$$restProps') {
171-
serialized = b.call('$.access_props', serialized);
170+
// If the binding is a prop, we need to deep read it because it could be fine-grained $state
171+
// from a runes-component, where mutations don't trigger an update on the prop as a whole.
172+
if (name === '$$props' || name === '$$restProps' || binding.kind === 'prop') {
173+
serialized = b.call('$.deep_read', serialized);
172174
}
173175

174176
sequence.push(serialized);

‎packages/svelte/src/internal/client/render.js

Copy file name to clipboardExpand all lines: packages/svelte/src/internal/client/render.js
+10-12Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ import {
4343
managed_effect,
4444
push,
4545
current_component_context,
46-
pop
46+
pop,
47+
deep_read
4748
} from './runtime.js';
4849
import {
4950
current_hydration_fragment,
@@ -1890,16 +1891,24 @@ export function action(dom, action, value_fn) {
18901891
effect(() => {
18911892
if (value_fn) {
18921893
const value = value_fn();
1894+
let needs_deep_read = false;
18931895
untrack(() => {
18941896
if (payload === undefined) {
18951897
payload = action(dom, value) || {};
1898+
needs_deep_read = !!payload?.update;
18961899
} else {
18971900
const update = payload.update;
18981901
if (typeof update === 'function') {
18991902
update(value);
19001903
}
19011904
}
19021905
});
1906+
// Action's update method is coarse-grained, i.e. when anything in the passed value changes, update.
1907+
// This works in legacy mode because of mutable_source being updated as a whole, but when using $state
1908+
// together with actions and mutation, it wouldn't notice the change without a deep read.
1909+
if (needs_deep_read) {
1910+
deep_read(value);
1911+
}
19031912
} else {
19041913
untrack(() => (payload = action(dom)));
19051914
}
@@ -2620,17 +2629,6 @@ export function unmount(component) {
26202629
fn?.();
26212630
}
26222631

2623-
/**
2624-
* @param {Record<string, unknown>} props
2625-
* @returns {void}
2626-
*/
2627-
export function access_props(props) {
2628-
for (const prop in props) {
2629-
// eslint-disable-next-line no-unused-expressions
2630-
props[prop];
2631-
}
2632-
}
2633-
26342632
/**
26352633
* @param {Record<string, any>} props
26362634
* @returns {Record<string, any>}

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

Copy file name to clipboardExpand all lines: packages/svelte/src/internal/client/runtime.js
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1985,11 +1985,13 @@ export function init() {
19851985
}
19861986

19871987
/**
1988+
* Deeply traverse an object and read all its properties
1989+
* so that they're all reactive in case this is `$state`
19881990
* @param {any} value
19891991
* @param {Set<any>} visited
19901992
* @returns {void}
19911993
*/
1992-
function deep_read(value, visited = new Set()) {
1994+
export function deep_read(value, visited = new Set()) {
19931995
if (typeof value === 'object' && value !== null && !visited.has(value)) {
19941996
visited.add(value);
19951997
for (let key in value) {

‎packages/svelte/src/internal/index.js

Copy file name to clipboardExpand all lines: packages/svelte/src/internal/index.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ export {
3838
inspect,
3939
unwrap,
4040
freeze,
41-
init
41+
init,
42+
deep_read
4243
} from './client/runtime.js';
4344
export { await_block as await } from './client/dom/blocks/await.js';
4445
export { if_block as if } from './client/dom/blocks/if.js';

‎packages/svelte/src/legacy/legacy-client.js

Copy file name to clipboardExpand all lines: packages/svelte/src/legacy/legacy-client.js
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ class Svelte4Component {
6666
* }} options
6767
*/
6868
constructor(options) {
69+
// Using proxy state here isn't completely mirroring the Svelte 4 behavior, because mutations to a property
70+
// cause fine-grained updates to only the places where that property is used, and not the entire property.
71+
// Reactive statements and actions (the things where this matters) are handling this properly regardless, so it should be fine in practise.
6972
const props = $.proxy({ ...(options.props || {}), $$events: this.#events }, false);
7073
this.#instance = (options.hydrate ? $.hydrate : $.mount)(options.component, {
7174
target: options.target,

‎packages/svelte/tests/runtime-legacy/samples/binding-backflow/_config.js

Copy file name to clipboardExpand all lines: packages/svelte/tests/runtime-legacy/samples/binding-backflow/_config.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@ export default test({
3434

3535
p = parents['reactive_mutate'];
3636
assert.deepEqual(p.value, { foo: 'kid' });
37-
assert.equal(p.updates.length, 1);
37+
assert.equal(p.updates.length, 2);
3838

3939
p = parents['init_update'];
4040
assert.deepEqual(p.value, { foo: 'kid' });
4141
assert.equal(p.updates.length, 2);
4242

4343
p = parents['init_mutate'];
4444
assert.deepEqual(p.value, { foo: 'kid' });
45-
assert.equal(p.updates.length, 1);
45+
assert.equal(p.updates.length, 2);
4646
}
4747
});

‎packages/svelte/tests/runtime-runes/samples/action-state-arg/_config.js

Copy file name to clipboard
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { test } from '../../test';
2+
import { tick } from 'svelte';
3+
4+
export default test({
5+
html: `<button>mutate</button><button>reassign</button><div>0</div>`,
6+
7+
async test({ assert, target }) {
8+
const [btn1, btn2] = target.querySelectorAll('button');
9+
10+
btn1.click();
11+
await tick();
12+
assert.htmlEqual(
13+
target.innerHTML,
14+
`<button>mutate</button><button>reassign</button><div>1</div>`
15+
);
16+
17+
btn2.click();
18+
await tick();
19+
assert.htmlEqual(
20+
target.innerHTML,
21+
`<button>mutate</button><button>reassign</button><div>2</div>`
22+
);
23+
}
24+
});

‎packages/svelte/tests/runtime-runes/samples/action-state-arg/main.svelte

Copy file name to clipboard
+16Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<script>
2+
let foo = $state({ count: 0 });
3+
let count = $state(0);
4+
5+
function action() {
6+
return {
7+
update(foo) {
8+
count = foo.count;
9+
}
10+
}
11+
}
12+
</script>
13+
14+
<button onclick={() => foo.count++}>mutate</button>
15+
<button onclick={() => foo = {...foo, count: foo.count + 1 }}>reassign</button>
16+
<div use:action={foo}>{count}</div>

‎packages/svelte/tests/runtime-runes/samples/fine-grained-prop-reactive-statement/_config.js

Copy file name to clipboard
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { test } from '../../test';
2+
import { tick } from 'svelte';
3+
4+
export default test({
5+
html: `<button>reassign</button><button>mutate</button><p>0 / 0</p>`,
6+
7+
async test({ assert, target }) {
8+
const [btn1, btn2] = target.querySelectorAll('button');
9+
10+
btn1.click();
11+
await tick();
12+
assert.htmlEqual(
13+
target.innerHTML,
14+
`<button>reassign</button><button>mutate</button><p>1 / 1</p>`
15+
);
16+
17+
btn2.click();
18+
await tick();
19+
assert.htmlEqual(
20+
target.innerHTML,
21+
`<button>reassign</button><button>mutate</button><p>2 / 2</p>`
22+
);
23+
}
24+
});

‎packages/svelte/tests/runtime-runes/samples/fine-grained-prop-reactive-statement/main.svelte

Copy file name to clipboard
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<script>
2+
import Old from './old.svelte';
3+
4+
let prop = $state({ count: 0 });
5+
</script>
6+
7+
<button onclick={() => prop = {...prop, count: prop.count + 1 }}>reassign</button>
8+
<button onclick={() => prop.count++}>mutate</button>
9+
<Old {prop}></Old>

‎packages/svelte/tests/runtime-runes/samples/fine-grained-prop-reactive-statement/old.svelte

Copy file name to clipboard
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<svelte:options runes={false} />
2+
<script>
3+
export let prop;
4+
let count_1 = prop.count;
5+
$: {
6+
count_1 = prop.count;
7+
}
8+
$: count_2 = prop.count;
9+
</script>
10+
11+
<p>{count_1} / {count_2}</p>

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.