Merge branch 'main' into props-bindable

props-bindable
Simon Holthausen 8 months ago
commit 6f8a4516f6

@ -0,0 +1,5 @@
---
"svelte": patch
---
feat: add support for webkitdirectory DOM boolean attribute

@ -0,0 +1,5 @@
---
"svelte": patch
---
fix: adjust scope parent for named slots

@ -0,0 +1,5 @@
---
"svelte": patch
---
fix: improve handling of unowned derived signals

@ -0,0 +1,5 @@
---
"svelte": patch
---
fix: ensure select value is updated upon select option removal

@ -1056,6 +1056,7 @@ export interface HTMLInputAttributes extends HTMLAttributes<HTMLInputElement> {
type?: HTMLInputTypeAttribute | undefined | null;
value?: any;
width?: number | string | undefined | null;
webkitdirectory?: boolean | undefined | null;
'on:change'?: ChangeEventHandler<HTMLInputElement> | undefined | null;
onchange?: ChangeEventHandler<HTMLInputElement> | undefined | null;

@ -158,26 +158,6 @@ function serialize_class_directives(class_directives, element_id, context, is_at
}
}
/**
*
* @param {string | null} spread_id
* @param {import('#compiler').RegularElement} node
* @param {import('../types.js').ComponentContext} context
* @param {import('estree').Identifier} node_id
*/
function add_select_to_spread_update(spread_id, node, context, node_id) {
if (spread_id !== null && node.name === 'select') {
context.state.update.push({
grouped: b.if(
b.binary('in', b.literal('value'), b.id(spread_id)),
b.block([
b.stmt(b.call('$.select_option', node_id, b.member(b.id(spread_id), b.id('value'))))
])
)
});
}
}
/**
* @param {import('#compiler').Binding[]} references
* @param {import('../types.js').ComponentContext} context
@ -223,6 +203,8 @@ function collect_transitive_dependencies(binding, seen = new Set()) {
* @param {import('../types.js').ComponentContext} context
*/
function setup_select_synchronization(value_binding, context) {
if (context.state.analysis.runes) return;
let bound = value_binding.expression;
while (bound.type === 'MemberExpression') {
bound = /** @type {import('estree').Identifier | import('estree').MemberExpression} */ (
@ -243,59 +225,49 @@ function setup_select_synchronization(value_binding, context) {
}
}
if (!context.state.analysis.runes) {
const invalidator = b.call(
'$.invalidate_inner_signals',
b.thunk(
b.block(
names.map((name) => {
const serialized = serialize_get_binding(b.id(name), context.state);
return b.stmt(serialized);
})
)
const invalidator = b.call(
'$.invalidate_inner_signals',
b.thunk(
b.block(
names.map((name) => {
const serialized = serialize_get_binding(b.id(name), context.state);
return b.stmt(serialized);
})
)
);
)
);
context.state.init.push(
b.stmt(
b.call(
'$.invalidate_effect',
b.thunk(
b.block([
b.stmt(
/** @type {import('estree').Expression} */ (context.visit(value_binding.expression))
),
b.stmt(invalidator)
])
)
context.state.init.push(
b.stmt(
b.call(
'$.invalidate_effect',
b.thunk(
b.block([
b.stmt(
/** @type {import('estree').Expression} */ (context.visit(value_binding.expression))
),
b.stmt(invalidator)
])
)
)
);
}
)
);
}
/**
* Serializes element attribute assignments that contain spreads to either only
* the init or the the init and update arrays, depending on whether or not the value is dynamic.
* Resulting code for static looks something like this:
* ```js
* $.spread_attributes(element, null, [...]);
* ```
* Resulting code for dynamic looks something like this:
* ```js
* let value;
* $.render_effect(() => {
* value = $.spread_attributes(element, value, [...])
* });
* ```
* Returns the id of the spread_attribute variable if spread isn't isolated, `null` otherwise.
* @param {Array<import('#compiler').Attribute | import('#compiler').SpreadAttribute>} attributes
* @param {import('../types.js').ComponentContext} context
* @param {import('#compiler').RegularElement} element
* @param {import('estree').Identifier} element_id
* @returns {string | null}
* @param {boolean} needs_select_handling
*/
function serialize_element_spread_attributes(attributes, context, element, element_id) {
function serialize_element_spread_attributes(
attributes,
context,
element,
element_id,
needs_select_handling
) {
let needs_isolation = false;
/** @type {import('estree').Expression[]} */
@ -317,8 +289,9 @@ function serialize_element_spread_attributes(attributes, context, element, eleme
const lowercase_attributes =
element.metadata.svg || is_custom_element_node(element) ? b.false : b.true;
const id = context.state.scope.generate('spread_attributes');
const isolated = b.stmt(
const standalone = b.stmt(
b.call(
'$.spread_attributes_effect',
element_id,
@ -327,32 +300,57 @@ function serialize_element_spread_attributes(attributes, context, element, eleme
b.literal(context.state.analysis.css.hash)
)
);
const inside_effect = b.stmt(
b.assignment(
'=',
b.id(id),
b.call(
'$.spread_attributes',
element_id,
b.id(id),
b.array(values),
lowercase_attributes,
b.literal(context.state.analysis.css.hash)
)
)
);
if (!needs_isolation || needs_select_handling) {
context.state.init.push(b.let(id));
}
// objects could contain reactive getters -> play it safe and always assume spread attributes are reactive
if (needs_isolation) {
context.state.update_effects.push(isolated);
return null;
if (needs_select_handling) {
context.state.update_effects.push(
b.stmt(b.call('$.render_effect', b.arrow([], b.block([inside_effect]))))
);
} else {
context.state.update_effects.push(standalone);
}
} else {
const id = context.state.scope.generate('spread_attributes');
context.state.init.push(b.let(id));
context.state.update.push({
singular: isolated,
grouped: b.stmt(
b.assignment(
'=',
b.id(id),
b.call(
'$.spread_attributes',
element_id,
b.id(id),
b.array(values),
lowercase_attributes,
b.literal(context.state.analysis.css.hash)
)
)
singular: needs_select_handling ? undefined : standalone,
grouped: inside_effect
});
}
if (needs_select_handling) {
context.state.init.push(
b.stmt(b.call('$.init_select', element_id, b.thunk(b.member(b.id(id), b.id('value')))))
);
context.state.update.push({
grouped: b.if(
b.binary('in', b.literal('value'), b.id(id)),
b.block([
// This ensures a one-way street to the DOM in case it's <select {value}>
// and not <select bind:value>. We need it in addition to $.init_select
// because the select value is not reflected as an attribute, so the
// mutation observer wouldn't notice.
b.stmt(b.call('$.select_option', element_id, b.member(b.id(id), b.id('value'))))
])
)
});
return id;
}
}
@ -644,27 +642,27 @@ function serialize_element_special_value_attribute(element, node_id, attribute,
)
);
const is_reactive = attribute.metadata.dynamic;
const needs_selected_call =
element === 'option' && (is_reactive || collect_parent_each_blocks(context).length > 0);
const needs_option_call = element === 'select' && is_reactive;
const is_select_with_value =
// attribute.metadata.dynamic would give false negatives because even if the value does not change,
// the inner options could still change, so we need to always treat it as reactive
element === 'select' && attribute.value !== true && !is_text_attribute(attribute);
const assignment = b.stmt(
needs_selected_call
is_select_with_value
? b.sequence([
inner_assignment,
// This ensures things stay in sync with the select binding
// in case of updates to the option value or new values appearing
b.call('$.selected', node_id)
// This ensures a one-way street to the DOM in case it's <select {value}>
// and not <select bind:value>. We need it in addition to $.init_select
// because the select value is not reflected as an attribute, so the
// mutation observer wouldn't notice.
b.call('$.select_option', node_id, value)
])
: needs_option_call
? b.sequence([
inner_assignment,
// This ensures a one-way street to the DOM in case it's <select {value}>
// and not <select bind:value>
b.call('$.select_option', node_id, value)
])
: inner_assignment
: inner_assignment
);
if (is_select_with_value) {
state.init.push(b.stmt(b.call('$.init_select', node_id, b.thunk(value))));
}
if (is_reactive) {
const id = state.scope.generate(`${node_id.name}_value`);
serialize_update_assignment(
@ -2083,11 +2081,15 @@ export const template_visitors = {
// Then do attributes
let is_attributes_reactive = false;
if (node.metadata.has_spread) {
const spread_id = serialize_element_spread_attributes(attributes, context, node, node_id);
if (child_metadata.namespace !== 'foreign') {
add_select_to_spread_update(spread_id, node, context, node_id);
}
is_attributes_reactive = spread_id !== null;
serialize_element_spread_attributes(
attributes,
context,
node,
node_id,
// If value binding exists, that one takes care of calling $.init_select
value_binding === null && node.name === 'select' && child_metadata.namespace !== 'foreign'
);
is_attributes_reactive = true;
} else {
for (const attribute of /** @type {import('#compiler').Attribute[]} */ (attributes)) {
if (is_event_attribute(attribute)) {

@ -400,9 +400,9 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
(attribute) => attribute.type === 'Attribute' && attribute.name === 'slot'
)
) {
// <div slot="..."> inherits the scope above the component, because slots are hella weird
scopes.set(child, state.scope);
visit(child);
// <div slot="..."> inherits the scope above the component unless the component is a named slot itself, because slots are hella weird
scopes.set(child, is_default_slot ? state.scope : scope);
visit(child, { scope: is_default_slot ? state.scope : scope });
} else {
if (child.type === 'ExpressionTag') {
// expression tag is a special case — we don't visit it directly, but via process_children,
@ -599,26 +599,38 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
},
AwaitBlock(node, context) {
context.next();
context.visit(node.expression);
if (node.pending) {
context.visit(node.pending);
}
if (node.then && node.value !== null) {
const then_scope = /** @type {Scope} */ (scopes.get(node.then));
const value_scope = context.state.scope.child();
for (const id of extract_identifiers(node.value)) {
then_scope.declare(id, 'normal', 'const');
value_scope.declare(id, 'normal', 'const');
if (node.then) {
context.visit(node.then);
if (node.value) {
const then_scope = /** @type {Scope} */ (scopes.get(node.then));
const value_scope = context.state.scope.child();
scopes.set(node.value, value_scope);
context.visit(node.value, { scope: value_scope });
for (const id of extract_identifiers(node.value)) {
then_scope.declare(id, 'normal', 'const');
value_scope.declare(id, 'normal', 'const');
}
}
scopes.set(node.value, value_scope);
}
if (node.catch && node.error !== null) {
const catch_scope = /** @type {Scope} */ (scopes.get(node.catch));
const error_scope = context.state.scope.child();
for (const id of extract_identifiers(node.error)) {
catch_scope.declare(id, 'normal', 'const');
error_scope.declare(id, 'normal', 'const');
if (node.catch) {
context.visit(node.catch);
if (node.error) {
const catch_scope = /** @type {Scope} */ (scopes.get(node.catch));
const error_scope = context.state.scope.child();
scopes.set(node.error, error_scope);
context.visit(node.error, { scope: error_scope });
for (const id of extract_identifiers(node.error)) {
catch_scope.declare(id, 'normal', 'const');
error_scope.declare(id, 'normal', 'const');
}
}
scopes.set(node.error, error_scope);
}
},

@ -83,7 +83,8 @@ export const DOMBooleanAttributes = [
'required',
'reversed',
'seamless',
'selected'
'selected',
'webkitdirectory'
];
export const namespace_svg = 'http://www.w3.org/2000/svg';

@ -1,4 +1,5 @@
import { effect } from '../../../reactivity/effects.js';
import { untrack } from '../../../runtime.js';
/**
* Selects the correct option(s) (depending on whether this is a multiple select)
@ -26,26 +27,43 @@ export function select_option(select, value, mounting) {
}
/**
* Finds the containing `<select>` element and potentially updates its `selected` state.
* @param {HTMLOptionElement} option
* @returns {void}
* Selects the correct option(s) if `value` is given,
* and then sets up a mutation observer to sync the
* current selection to the dom when it changes. Such
* changes could for example occur when options are
* inside an `#each` block.
* @template V
* @param {HTMLSelectElement} select
* @param {() => V} [get_value]
*/
export function selected(option) {
// Inside an effect because the element might not be connected
// to the parent <select> yet when this is called
export function init_select(select, get_value) {
effect(() => {
var select = option.parentNode;
while (select != null) {
if (select.nodeName === 'SELECT') break;
select = select.parentNode;
if (get_value) {
select_option(select, untrack(get_value));
}
// @ts-ignore
if (select != null && option.__value === select.__value) {
// never set to false, since this causes browser to select default option
option.selected = true;
}
var observer = new MutationObserver(() => {
// @ts-ignore
var value = select.__value;
select_option(select, value);
// Deliberately don't update the potential binding value,
// the model should be preserved unless explicitly changed
});
observer.observe(select, {
// Listen to option element changes
childList: true,
subtree: true, // because of <optgroup>
// Listen to option element value attribute changes
// (doesn't get notified of select value changes,
// because that property is not reflected as an attribute)
attributes: true,
attributeFilter: ['value']
});
return () => {
observer.disconnect();
};
});
}
@ -78,6 +96,7 @@ export function bind_select_value(select, get_value, update) {
var value = get_value();
select_option(select, value, mounting);
// Mounting and value undefined -> take selection from dom
if (mounting && value === undefined) {
/** @type {HTMLOptionElement | null} */
var selected_option = select.querySelector(':checked');
@ -91,6 +110,9 @@ export function bind_select_value(select, get_value, update) {
select.__value = value;
mounting = false;
});
// don't pass get_value, we already initialize it in the effect above
init_select(select);
}
/**

@ -41,7 +41,7 @@ export function derived(fn) {
/** @type {import('#client').DerivedDebug} */ (signal).inspect = new Set();
}
if (current_reaction !== null) {
if (current_reaction !== null && (current_reaction.f & DERIVED) !== 0) {
if (current_reaction.deriveds === null) {
current_reaction.deriveds = [signal];
} else {

@ -14,9 +14,11 @@ export default test({
<p>selected: two</p>
`,
test({ assert, component, target }) {
async test({ assert, component, target }) {
component.items = ['one', 'two', 'three'];
await Promise.resolve(); // mutation observer
const options = target.querySelectorAll('option');
assert.ok(!options[0].selected);
assert.ok(options[1].selected);

@ -19,9 +19,11 @@ export default test({
<p>selected: two</p>
`,
test({ assert, component, target }) {
async test({ assert, component, target }) {
component.items = ['one', 'two', 'three'];
await Promise.resolve(); // mutation observer
const options = target.querySelectorAll('option');
assert.ok(!options[0].selected);
assert.ok(options[1].selected);

@ -0,0 +1,12 @@
import { test } from '../../test';
export default test({
async test({ assert, target }) {
await new Promise((r) => setTimeout(r, 200)); // wait for await block to resolve
const options = target.querySelectorAll('option');
assert.ok(!options[0].selected);
assert.ok(options[1].selected);
assert.ok(!options[2].selected);
}
});

@ -0,0 +1,19 @@
<script>
let promise = getNumbers();
let selected = 2;
async function getNumbers() {
await new Promise(resolve => setTimeout(resolve, 100));
return [1, 2, 3];
}
</script>
<select bind:value={selected}>
{#await promise}
<option>-1</option>
{:then numbers}
{#each numbers as number}
<option>{number}</option>
{/each}
{/await}
</select>

@ -0,0 +1,35 @@
import { ok, test } from '../../test';
// test select binding behavior when a selected option is removed
export default test({
skip_if_ssr: 'permanent',
html: `<p>selected: a</p><select><option value="a">a</option><option value="b">b</option><option value="c">c</option></select>`,
async test({ assert, component, target }) {
const select = target.querySelector('select');
ok(select);
const options = target.querySelectorAll('option');
// first option should be selected by default since no value was bound
assert.equal(component.selected, 'a');
assert.equal(select.value, 'a');
assert.ok(options[0].selected);
// remove the selected item, so the bound value no longer matches anything
component.items = ['b', 'c'];
// There's a MutationObserver
await Promise.resolve();
// now no option should be selected
assert.equal(select.value, '');
assert.equal(select.selectedIndex, -1);
// model of selected value should be kept around, even if it is not in the list
assert.htmlEqual(
target.innerHTML,
`<p>selected: a</p><select><option value="b">b</option><option value="c">c</option></select>`
);
}
});

@ -0,0 +1,12 @@
<script>
export let selected;
export let items = ['a', 'b', 'c'];
</script>
<p>selected: {selected}</p>
<select bind:value={selected}>
{#each items as letter}
<option>{letter}</option>
{/each}
</select>

@ -0,0 +1,9 @@
<script>
export let text;
</script>
<div>
{text}
<hr />
<slot name="footer" />
</div>

@ -0,0 +1,7 @@
import { test } from '../../test';
export default test({
html: `
<div>hello world <hr> <div slot="footer">hello world</div></div>
`
});

@ -0,0 +1,12 @@
<script>
import Nested from "./Nested.svelte"
import Nested2 from "./Nested2.svelte"
</script>
<Nested>
<Nested2 slot="inner" let:text {text}>
<div slot="footer">
{text}
</div>
</Nested2>
</Nested>

@ -0,0 +1,16 @@
import { test } from '../../test';
export default test({
async test({ assert, target }) {
await target.querySelector('button')?.click();
await Promise.resolve();
const options = target.querySelectorAll('option');
assert.equal(options[0].selected, false);
assert.equal(options[1].selected, true);
assert.equal(options[2].selected, false);
assert.equal(options[3].selected, false);
assert.equal(options[4].selected, true);
assert.equal(options[5].selected, false);
}
});

@ -0,0 +1,33 @@
<script>
let value = 'bar';
let value_bound = 'bar';
let options = {};
function loadOptions() {
options = {
foo: 'Foo',
bar: 'Bar',
baz: 'Baz',
};
}
</script>
<select {value}>
{#each Object.entries(options) as [key, value] (key)}
<option value={key}>
{value}
</option>
{/each}
</select>
<select bind:value={value_bound}>
{#each Object.entries(options) as [key, value] (key)}
<option value={key}>
{value}
</option>
{/each}
</select>
<button on:click={loadOptions}>
Load options
</button>

@ -0,0 +1,21 @@
import { test } from '../../test';
const items = [{ id: 'a' }, { id: 'b' }];
export default test({
get props() {
return { foo: [items[0]], items };
},
test({ assert, component, target }) {
const options = target.querySelectorAll('option');
assert.equal(options[0].selected, true);
assert.equal(options[1].selected, false);
component.foo = [component.items[1]];
assert.equal(options[0].selected, false);
assert.equal(options[1].selected, true);
}
});

@ -0,0 +1,9 @@
<script>
export let foo;
export let items;
</script>
<select value="{foo}" multiple>
<option value='{items[0]}'>a</option>
<option value='{items[1]}'>b</option>
</select>

@ -0,0 +1,30 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target }) {
let [btn1, btn2] = target.querySelectorAll('button');
flushSync(() => {
btn1.click();
});
flushSync(() => {
btn2.click();
});
assert.htmlEqual(
target.innerHTML,
`<button>Activate</button><button>Toggle</button>\nneighba\nneighba`
);
flushSync(() => {
btn2.click();
});
assert.htmlEqual(
target.innerHTML,
`<button>Activate</button><button>Toggle</button>\nzeeba\nzeeba`
);
}
});

@ -0,0 +1,49 @@
<script>
import { untrack } from 'svelte';
class Model {
data = $state();
constructor(data) {
this.data = data;
}
name = $derived(this.data?.name);
source = $derived(this.data?.source);
toggle() {
this.data.name = this.data.name === 'zeeba' ? 'neighba' : 'zeeba';
}
}
let model = $state(new Model({ name: 'zeeba', source: 'initial' }));
let setModel = (source) => {
let next = new Model({ name: 'zeeba', source });
model = next;
}
let needsSet = $state(false);
$effect(() => {
if(needsSet) {
setModel('effect');
untrack(() => {
needsSet = false;
});
}
});
let setWithEffect = () => {
needsSet = true;
};
let toggle = () => {
model.toggle();
}
</script>
<button onclick={setWithEffect}>Activate</button>
<button onclick={toggle}>Toggle</button>
{model.name}
{model.data.name}
Loading…
Cancel
Save