Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure props passed to components via mount are updateable #14210

Merged
merged 8 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fresh-pigs-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: ensure props passed to components via mount are updateable
1 change: 1 addition & 0 deletions packages/svelte/src/internal/client/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ export const EFFECT_HAS_DERIVED = 1 << 19;

export const STATE_SYMBOL = Symbol('$state');
export const STATE_SYMBOL_METADATA = Symbol('$state metadata');
export const LEGACY_PROPS = Symbol('legacy props');
export const LOADING_ATTR_SYMBOL = Symbol('');
20 changes: 18 additions & 2 deletions packages/svelte/src/internal/client/reactivity/props.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ import {
} from '../runtime.js';
import { safe_equals } from './equality.js';
import * as e from '../errors.js';
import { BRANCH_EFFECT, LEGACY_DERIVED_PROP, ROOT_EFFECT } from '../constants.js';
import {
BRANCH_EFFECT,
LEGACY_DERIVED_PROP,
LEGACY_PROPS,
ROOT_EFFECT,
STATE_SYMBOL
} from '../constants.js';
import { proxy } from '../proxy.js';
import { capture_store_binding } from './store.js';
import { legacy_mode_flag } from '../../flags/index.js';
Expand Down Expand Up @@ -209,6 +215,9 @@ const spread_props_handler = {
}
},
has(target, key) {
// To prevent a false positive `is_entry_props` in the `prop` function
if (key === STATE_SYMBOL || key === LEGACY_PROPS) return false;

for (let p of target.props) {
if (is_function(p)) p = p();
if (p != null && key in p) return true;
Expand Down Expand Up @@ -282,7 +291,14 @@ export function prop(props, key, flags, fallback) {
} else {
prop_value = /** @type {V} */ (props[key]);
}
var setter = get_descriptor(props, key)?.set;

// Can be the case when someone does `mount(Component, props)` with `let props = $state({...})`
// or `createClassComponent(Component, props)`
var is_entry_props = STATE_SYMBOL in props || LEGACY_PROPS in props;

var setter =
get_descriptor(props, key)?.set ??
(is_entry_props && bindable && key in props ? (v) => (props[key] = v) : undefined);

var fallback_value = /** @type {V} */ (fallback);
var fallback_dirty = true;
Expand Down
11 changes: 1 addition & 10 deletions packages/svelte/src/internal/client/validate.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,11 @@
import { dev_current_component_function, untrack } from './runtime.js';
import { dev_current_component_function } from './runtime.js';
import { get_descriptor, is_array } from '../shared/utils.js';
import * as e from './errors.js';
import { FILENAME } from '../../constants.js';
import { render_effect } from './reactivity/effects.js';
import * as w from './warnings.js';
import { capture_store_binding } from './reactivity/store.js';

/** regex of all html void element names */
const void_element_names =
/^(?:area|base|br|col|command|embed|hr|img|input|keygen|link|meta|param|source|track|wbr)$/;

/** @param {string} tag */
function is_void(tag) {
return void_element_names.test(tag) || tag.toLowerCase() === '!doctype';
}

/**
* @param {() => any} collection
* @param {(item: any, index: number) => string} key_fn
Expand Down
7 changes: 5 additions & 2 deletions packages/svelte/src/legacy/legacy-client.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @import { ComponentConstructorOptions, ComponentType, SvelteComponent, Component } from 'svelte' */
import { DIRTY, MAYBE_DIRTY } from '../internal/client/constants.js';
import { DIRTY, LEGACY_PROPS, MAYBE_DIRTY } from '../internal/client/constants.js';
import { user_pre_effect } from '../internal/client/reactivity/effects.js';
import { mutable_source, set } from '../internal/client/reactivity/sources.js';
import { hydrate, mount, unmount } from '../internal/client/render.js';
Expand Down Expand Up @@ -89,7 +89,7 @@ class Svelte4Component {
};

// Replicate coarse-grained props through a proxy that has a version source for
// each property, which is increment on updates to the property itself. Do not
// each property, which is incremented on updates to the property itself. Do not
// use our $state proxy because that one has fine-grained reactivity.
const props = new Proxy(
{ ...(options.props || {}), $$events: {} },
Expand All @@ -98,6 +98,9 @@ class Svelte4Component {
return get(sources.get(prop) ?? add_source(prop, Reflect.get(target, prop)));
},
has(target, prop) {
// Necessary to not throw "invalid binding" validation errors on the component side
if (prop === LEGACY_PROPS) return true;

get(sources.get(prop) ?? add_source(prop, Reflect.get(target, prop)));
return Reflect.has(target, prop);
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { flushSync } from 'svelte';
import { test } from '../../test';

export default test({
test({ assert, target }) {
assert.htmlEqual(
target.innerHTML,
// The buz fallback does not propagate back up
`
<button>reset</button> foo baz
<div><button>update</button> foo bar baz buz</div>
<div><button>update</button> foo bar baz buz</div>
`
);

const [btn1, btn2, btn3] = target.querySelectorAll('button');

btn2.click();
btn3.click();
flushSync();
assert.htmlEqual(
target.innerHTML,
// bar is not set in the parent because it's a readonly property
// baz is not set in the parent because while it's a bindable property,
// it wasn't set initially so it's treated as a readonly proeprty
`
<button>reset</button> foo 3
<div><button>update</button> 1 2 3 4</div>
<div><button>update</button> 1 2 3 4</div>
`
);

btn1.click();
flushSync();
assert.htmlEqual(
target.innerHTML,
// Because foo is a readonly property, component.svelte diverges locally from it,
// and the passed in property keeps the initial value of foo. This is why it stays
// at 1, because foo is not updated to a different value.
`
<button>reset</button> foo bar baz buz
<div><button>update</button> 1 bar baz buz</div>
<div><button>update</button> 1 bar baz buz</div>
`
);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<script>
let { foo, bar = 'bar', baz = $bindable(), buz = $bindable('buz') } = $props();
</script>

<button
onclick={() => {
foo = '1';
bar = '2';
baz = '3';
buz = '4';
}}>update</button
>

{foo}
{bar}
{baz}
{buz}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<script>
import { createClassComponent } from 'svelte/legacy';
import Component from './component.svelte';
import { mount, onMount } from 'svelte';

let div1, div2;
let legacy;
const props = $state({ foo: 'foo', baz: 'baz' });

onMount(() => {
legacy = createClassComponent({
component: Component,
target: div1,
props: { foo: 'foo', baz: 'baz' }
});
mount(Component, { target: div2, props });
});
</script>

<button
onclick={() => {
legacy.$set({ foo: 'foo', bar: 'bar', baz: 'baz', buz: 'buz' });
props.foo = 'foo';
props.bar = 'bar';
props.baz = 'baz';
props.buz = 'buz';
}}>reset</button
> {props.foo} {props.bar} {props.baz} {props.buz}
<div bind:this={div1}></div>
<div bind:this={div2}></div>
Loading