Skip to content

Commit

Permalink
fix: ignore value attribute on select during SSR (#11724)
Browse files Browse the repository at this point in the history
The value attribute on select elements does nothing - it does not influence the initial value (in SSR that's the job of the `selected` attribute on an option element), updating it does not influence the current value either. Instead of rendering it out and then removing it on hydration (which is costly because the mutation causes work) we just don't render it in SSR.

No test/changeset because no change in behavior.
  • Loading branch information
dummdidumm committed May 22, 2024
1 parent 6d2f1a4 commit d590cd8
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1990,7 +1990,7 @@ export const template_visitors = {
child_metadata.bound_contenteditable = true;
}

if (needs_input_reset && (node.name === 'input' || node.name === 'select')) {
if (needs_input_reset && node.name === 'input') {
context.state.init.push(b.stmt(b.call('$.remove_input_attr_defaults', context.state.node)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1851,18 +1851,27 @@ function serialize_element_attributes(node, context) {

for (const attribute of node.attributes) {
if (attribute.type === 'Attribute') {
if (attribute.name === 'value' && node.name === 'textarea') {
if (
attribute.value !== true &&
attribute.value[0].type === 'Text' &&
regex_starts_with_newline.test(attribute.value[0].data)
) {
// Two or more leading newlines are required to restore the leading newline immediately after `<textarea>`.
// see https://html.spec.whatwg.org/multipage/syntax.html#element-restrictions
// also see related code in analysis phase
attribute.value[0].data = '\n' + attribute.value[0].data;
if (attribute.name === 'value') {
if (node.name === 'textarea') {
if (
attribute.value !== true &&
attribute.value[0].type === 'Text' &&
regex_starts_with_newline.test(attribute.value[0].data)
) {
// Two or more leading newlines are required to restore the leading newline immediately after `<textarea>`.
// see https://html.spec.whatwg.org/multipage/syntax.html#element-restrictions
// also see related code in analysis phase
attribute.value[0].data = '\n' + attribute.value[0].data;
}
content = {
escape: true,
expression: serialize_attribute_value(attribute.value, context)
};
} else if (node.name !== 'select') {
// omit value attribute for select elements, it's irrelevant for the initially selected value and has no
// effect on the selected value after the user interacts with the select element (the value _property_ does, but not the attribute)
attributes.push(attribute);
}
content = { escape: true, expression: serialize_attribute_value(attribute.value, context) };

// omit event handlers except for special cases
} else if (is_event_attribute(attribute)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { LOADING_ATTR_SYMBOL } from '../../constants.js';
/**
* The value/checked attribute in the template actually corresponds to the defaultValue property, so we need
* to remove it upon hydration to avoid a bug when someone resets the form value.
* @param {HTMLInputElement | HTMLSelectElement} dom
* @param {HTMLInputElement} dom
* @returns {void}
*/
export function remove_input_attr_defaults(dom) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ export default test({
<p>selected: two</p>
`,

ssrHtml: `
<select value="two"></select>
<p>selected: two</p>
`,

async test({ assert, component, target }) {
component.items = ['one', 'two', 'three'];

Expand Down

0 comments on commit d590cd8

Please sign in to comment.