Skip to content

Commit e379319

Browse files
authored
fix: ensure props passed to components via mount are updateable (#14210)
The detection whether or not props are writable is buggy; it doesn't take into account the case when instantiating components via `mount` or legacy-`new` Fixes #14161 This posed an interesting question: What to do about (non-)`bind:`able properties? The answer I arrived on was: Treat it as if everything that _can_ be bound _is_ treated as bound, and everything else as readonly. In other words, if you're reassigning a prop, it will diverge from the passed in props if it's not bindable or not set in the parent, otherwise it will mutate the passed in props. I think that makes the most sense, given that you can't control this behavior from the outside.
1 parent 7fd3774 commit e379319

File tree

8 files changed

+124
-14
lines changed

8 files changed

+124
-14
lines changed

.changeset/fresh-pigs-divide.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: ensure props passed to components via mount are updateable

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@ export const EFFECT_HAS_DERIVED = 1 << 19;
2222

2323
export const STATE_SYMBOL = Symbol('$state');
2424
export const STATE_SYMBOL_METADATA = Symbol('$state metadata');
25+
export const LEGACY_PROPS = Symbol('legacy props');
2526
export const LOADING_ATTR_SYMBOL = Symbol('');

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,13 @@ import {
2020
} from '../runtime.js';
2121
import { safe_equals } from './equality.js';
2222
import * as e from '../errors.js';
23-
import { BRANCH_EFFECT, LEGACY_DERIVED_PROP, ROOT_EFFECT } from '../constants.js';
23+
import {
24+
BRANCH_EFFECT,
25+
LEGACY_DERIVED_PROP,
26+
LEGACY_PROPS,
27+
ROOT_EFFECT,
28+
STATE_SYMBOL
29+
} from '../constants.js';
2430
import { proxy } from '../proxy.js';
2531
import { capture_store_binding } from './store.js';
2632
import { legacy_mode_flag } from '../../flags/index.js';
@@ -209,6 +215,9 @@ const spread_props_handler = {
209215
}
210216
},
211217
has(target, key) {
218+
// To prevent a false positive `is_entry_props` in the `prop` function
219+
if (key === STATE_SYMBOL || key === LEGACY_PROPS) return false;
220+
212221
for (let p of target.props) {
213222
if (is_function(p)) p = p();
214223
if (p != null && key in p) return true;
@@ -282,7 +291,14 @@ export function prop(props, key, flags, fallback) {
282291
} else {
283292
prop_value = /** @type {V} */ (props[key]);
284293
}
285-
var setter = get_descriptor(props, key)?.set;
294+
295+
// Can be the case when someone does `mount(Component, props)` with `let props = $state({...})`
296+
// or `createClassComponent(Component, props)`
297+
var is_entry_props = STATE_SYMBOL in props || LEGACY_PROPS in props;
298+
299+
var setter =
300+
get_descriptor(props, key)?.set ??
301+
(is_entry_props && bindable && key in props ? (v) => (props[key] = v) : undefined);
286302

287303
var fallback_value = /** @type {V} */ (fallback);
288304
var fallback_dirty = true;

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

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,11 @@
1-
import { dev_current_component_function, untrack } from './runtime.js';
1+
import { dev_current_component_function } from './runtime.js';
22
import { get_descriptor, is_array } from '../shared/utils.js';
33
import * as e from './errors.js';
44
import { FILENAME } from '../../constants.js';
55
import { render_effect } from './reactivity/effects.js';
66
import * as w from './warnings.js';
77
import { capture_store_binding } from './reactivity/store.js';
88

9-
/** regex of all html void element names */
10-
const void_element_names =
11-
/^(?:area|base|br|col|command|embed|hr|img|input|keygen|link|meta|param|source|track|wbr)$/;
12-
13-
/** @param {string} tag */
14-
function is_void(tag) {
15-
return void_element_names.test(tag) || tag.toLowerCase() === '!doctype';
16-
}
17-
189
/**
1910
* @param {() => any} collection
2011
* @param {(item: any, index: number) => string} key_fn

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/** @import { ComponentConstructorOptions, ComponentType, SvelteComponent, Component } from 'svelte' */
2-
import { DIRTY, MAYBE_DIRTY } from '../internal/client/constants.js';
2+
import { DIRTY, LEGACY_PROPS, MAYBE_DIRTY } from '../internal/client/constants.js';
33
import { user_pre_effect } from '../internal/client/reactivity/effects.js';
44
import { mutable_source, set } from '../internal/client/reactivity/sources.js';
55
import { hydrate, mount, unmount } from '../internal/client/render.js';
@@ -89,7 +89,7 @@ class Svelte4Component {
8989
};
9090

9191
// Replicate coarse-grained props through a proxy that has a version source for
92-
// each property, which is increment on updates to the property itself. Do not
92+
// each property, which is incremented on updates to the property itself. Do not
9393
// use our $state proxy because that one has fine-grained reactivity.
9494
const props = new Proxy(
9595
{ ...(options.props || {}), $$events: {} },
@@ -98,6 +98,9 @@ class Svelte4Component {
9898
return get(sources.get(prop) ?? add_source(prop, Reflect.get(target, prop)));
9999
},
100100
has(target, prop) {
101+
// Necessary to not throw "invalid binding" validation errors on the component side
102+
if (prop === LEGACY_PROPS) return true;
103+
101104
get(sources.get(prop) ?? add_source(prop, Reflect.get(target, prop)));
102105
return Reflect.has(target, prop);
103106
},
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
test({ assert, target }) {
6+
assert.htmlEqual(
7+
target.innerHTML,
8+
// The buz fallback does not propagate back up
9+
`
10+
<button>reset</button> foo baz
11+
<div><button>update</button> foo bar baz buz</div>
12+
<div><button>update</button> foo bar baz buz</div>
13+
`
14+
);
15+
16+
const [btn1, btn2, btn3] = target.querySelectorAll('button');
17+
18+
btn2.click();
19+
btn3.click();
20+
flushSync();
21+
assert.htmlEqual(
22+
target.innerHTML,
23+
// bar is not set in the parent because it's a readonly property
24+
// baz is not set in the parent because while it's a bindable property,
25+
// it wasn't set initially so it's treated as a readonly proeprty
26+
`
27+
<button>reset</button> foo 3
28+
<div><button>update</button> 1 2 3 4</div>
29+
<div><button>update</button> 1 2 3 4</div>
30+
`
31+
);
32+
33+
btn1.click();
34+
flushSync();
35+
assert.htmlEqual(
36+
target.innerHTML,
37+
// Because foo is a readonly property, component.svelte diverges locally from it,
38+
// and the passed in property keeps the initial value of foo. This is why it stays
39+
// at 1, because foo is not updated to a different value.
40+
`
41+
<button>reset</button> foo bar baz buz
42+
<div><button>update</button> 1 bar baz buz</div>
43+
<div><button>update</button> 1 bar baz buz</div>
44+
`
45+
);
46+
}
47+
});
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<script>
2+
let { foo, bar = 'bar', baz = $bindable(), buz = $bindable('buz') } = $props();
3+
</script>
4+
5+
<button
6+
onclick={() => {
7+
foo = '1';
8+
bar = '2';
9+
baz = '3';
10+
buz = '4';
11+
}}>update</button
12+
>
13+
14+
{foo}
15+
{bar}
16+
{baz}
17+
{buz}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<script>
2+
import { createClassComponent } from 'svelte/legacy';
3+
import Component from './component.svelte';
4+
import { mount, onMount } from 'svelte';
5+
6+
let div1, div2;
7+
let legacy;
8+
const props = $state({ foo: 'foo', baz: 'baz' });
9+
10+
onMount(() => {
11+
legacy = createClassComponent({
12+
component: Component,
13+
target: div1,
14+
props: { foo: 'foo', baz: 'baz' }
15+
});
16+
mount(Component, { target: div2, props });
17+
});
18+
</script>
19+
20+
<button
21+
onclick={() => {
22+
legacy.$set({ foo: 'foo', bar: 'bar', baz: 'baz', buz: 'buz' });
23+
props.foo = 'foo';
24+
props.bar = 'bar';
25+
props.baz = 'baz';
26+
props.buz = 'buz';
27+
}}>reset</button
28+
> {props.foo} {props.bar} {props.baz} {props.buz}
29+
<div bind:this={div1}></div>
30+
<div bind:this={div2}></div>

0 commit comments

Comments
 (0)