Skip to content

Commit 7df4505

Browse files
committed
fix: runtime valueless option selection
1 parent e69e552 commit 7df4505

File tree

6 files changed

+67
-23
lines changed

6 files changed

+67
-23
lines changed

packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,19 +137,35 @@ export function RegularElement(node, context) {
137137
state.template.push(b.const(id, body));
138138
}
139139

140-
// if this is a `<textarea>` value or a contenteditable binding, we only add
141-
// the body if the attribute/binding is falsy
140+
// we need the body if:
141+
142+
// - it's a <textarea> or a contenteditable element...we will add it ad the inner template in case the value of value is falsy
143+
// - it's a valueless <option> element...we will check the body value at runtime to determine the implicit value selection
142144
const inner_state = { ...state, template: [], init: [] };
143145
process_children(trimmed, { ...context, state: inner_state });
144146

145-
// Use the body expression as the body if it's truthy, otherwise use the inner template
146-
state.template.push(
147-
b.if(
148-
id,
149-
b.block(build_template([id])),
150-
b.block([...inner_state.init, ...build_template(inner_state.template)])
151-
)
152-
);
147+
if (node.name === 'option') {
148+
// in case of a valueless `<option>` element, $$body is a function that accepts the children...internally it
149+
// will run the children to get the value of the body at runtime since it's needed to for the implicit value
150+
// selection
151+
state.template.push(
152+
b.stmt(
153+
b.call(
154+
id,
155+
b.thunk(b.block([...inner_state.init, ...build_template(inner_state.template)]))
156+
)
157+
)
158+
);
159+
} else {
160+
// Use the body expression as the body if it's truthy, otherwise use the inner template
161+
state.template.push(
162+
b.if(
163+
id,
164+
b.block(build_template([id])),
165+
b.block([...inner_state.init, ...build_template(inner_state.template)])
166+
)
167+
);
168+
}
153169
}
154170

155171
if (select_with_value) {

packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -305,14 +305,7 @@ export function build_element_attributes(node, context) {
305305
}
306306

307307
if (!option_has_value && node.name === 'option') {
308-
context.state.template.push(
309-
b.call(
310-
'$.maybe_selected',
311-
b.id('$$payload'),
312-
// this should be fine for the moment since option can only contain text and expressions
313-
build_attribute_value(/** @type {*} */ (node.fragment.nodes), context)
314-
)
315-
);
308+
content = b.call('$.valueless_option', b.id('$$payload'));
316309
}
317310

318311
if (events_to_capture.size !== 0) {

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,3 +544,24 @@ export function derived(fn) {
544544
export function maybe_selected(payload, value) {
545545
return value === payload.select_value ? ' selected' : '';
546546
}
547+
548+
/**
549+
* @param {Payload} payload
550+
* @returns {(child: () => void) => void}
551+
*/
552+
export function valueless_option(payload) {
553+
return (child) => {
554+
// store the initial payload before executing the child
555+
let initial_payload = payload.out;
556+
// execute the child to get the runtime body of the option
557+
child();
558+
// remove the initial payload and eventual hydration comments
559+
let body = payload.out.substring(initial_payload.length).replace(/<!---->/g, '');
560+
// check the value of the body with the select_value
561+
if (body === payload.select_value) {
562+
// substring the initial payload to remove the last character (the closing `>`)
563+
// append selected and the body (the closing tag will be added later)
564+
payload.out = initial_payload.substring(0, initial_payload.length - 1) + ' selected>' + body;
565+
}
566+
};
567+
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
<select><option value="">--Please choose an option--</option><option value="dog" selected>Dog</option><option value="cat">Cat</option></select>
1+
<select><option>--Please choose an option--</option><option selected>dog</option><option>cat</option></select>
Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
<select value="dog">
2-
<option>{@render option("--Please choose an option--")}</option>
3-
<option>{@render option("dog")}</option>
4-
<option>{@render option("cat")}</option>
2+
<option>
3+
{@render option("--Please choose an option--")}
4+
</option>
5+
<option>
6+
{@render option("dog")}
7+
</option>
8+
<option>
9+
{@render option("cat")}
10+
</option>
511
</select>
612

713
{#snippet option(val)}{val}{/snippet}

packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/server/index.svelte.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,13 @@ import * as $ from 'svelte/internal/server';
33
export default function Skip_static_subtree($$payload, $$props) {
44
let { title, content } = $$props;
55

6-
$$payload.out += `<header><nav><a href="/">Home</a> <a href="/away">Away</a></nav></header> <main><h1>${$.escape(title)}</h1> <div class="static"><p>we don't need to traverse these nodes</p></div> <p>or</p> <p>these</p> <p>ones</p> ${$.html(content)} <p>these</p> <p>trailing</p> <p>nodes</p> <p>can</p> <p>be</p> <p>completely</p> <p>ignored</p></main> <cant-skip><custom-elements with="attributes"></custom-elements></cant-skip> <div><input autofocus/></div> <div><source muted/></div> <select><option value="a"${$.maybe_selected($$payload, 'a')}${$.maybe_selected($$payload, 'a')}>a</option></select> <img src="..." alt="" loading="lazy"/> <div><img src="..." alt="" loading="lazy"/></div>`;
6+
$$payload.out += `<header><nav><a href="/">Home</a> <a href="/away">Away</a></nav></header> <main><h1>${$.escape(title)}</h1> <div class="static"><p>we don't need to traverse these nodes</p></div> <p>or</p> <p>these</p> <p>ones</p> ${$.html(content)} <p>these</p> <p>trailing</p> <p>nodes</p> <p>can</p> <p>be</p> <p>completely</p> <p>ignored</p></main> <cant-skip><custom-elements with="attributes"></custom-elements></cant-skip> <div><input autofocus/></div> <div><source muted/></div> <select><option value="a"${$.maybe_selected($$payload, 'a')}>`;
7+
8+
const $$body = $.valueless_option($$payload);
9+
10+
$$body(() => {
11+
$$payload.out += `a`;
12+
});
13+
14+
$$payload.out += `</option></select> <img src="..." alt="" loading="lazy"/> <div><img src="..." alt="" loading="lazy"/></div>`;
715
}

0 commit comments

Comments
 (0)