Skip to content
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/nasty-comics-play.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: SSR regression of processing attributes of `<select>` and `<option>`
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ import { is_void } from '../../../../../utils.js';
import { dev, locator } from '../../../../state.js';
import * as b from '#compiler/builders';
import { clean_nodes, determine_namespace_for_children } from '../../utils.js';
import { build_element_attributes, build_spread_object } from './shared/element.js';
import { build_element_attributes, prepare_element_spread_object } from './shared/element.js';
import {
process_children,
build_template,
build_attribute_value,
create_child_block,
PromiseOptimiser
} from './shared/utils.js';
Expand All @@ -37,9 +36,27 @@ export function RegularElement(node, context) {

const optimiser = new PromiseOptimiser();

state.template.push(b.literal(`<${node.name}`));
const body = build_element_attributes(node, { ...context, state }, optimiser.transform);
state.template.push(b.literal(node_is_void ? '/>' : '>')); // add `/>` for XHTML compliance
// If this element needs special handling (like <select value> / <option>),
// avoid calling build_element_attributes here to prevent evaluating/awaiting
// attribute expressions twice. We'll handle attributes in the special branch.
const is_select_special =
node.name === 'select' &&
node.attributes.some(
(attribute) =>
((attribute.type === 'Attribute' || attribute.type === 'BindDirective') &&
attribute.name === 'value') ||
attribute.type === 'SpreadAttribute'
);
const is_option_special = node.name === 'option';
const is_special = is_select_special || is_option_special;

let body = /** @type {Expression | null} */ (null);
if (!is_special) {
// only open the tag in the non-special path
state.template.push(b.literal(`<${node.name}`));
body = build_element_attributes(node, { ...context, state }, optimiser.transform);
state.template.push(b.literal(node_is_void ? '/>' : '>')); // add `/>` for XHTML compliance
}

if ((node.name === 'script' || node.name === 'style') && node.fragment.nodes.length === 1) {
state.template.push(
Expand Down Expand Up @@ -95,27 +112,7 @@ export function RegularElement(node, context) {
);
}

if (
node.name === 'select' &&
node.attributes.some(
(attribute) =>
((attribute.type === 'Attribute' || attribute.type === 'BindDirective') &&
attribute.name === 'value') ||
attribute.type === 'SpreadAttribute'
)
) {
const attributes = build_spread_object(
node,
node.attributes.filter(
(attribute) =>
attribute.type === 'Attribute' ||
attribute.type === 'BindDirective' ||
attribute.type === 'SpreadAttribute'
),
context,
optimiser.transform
);

if (is_select_special) {
const inner_state = { ...state, template: [], init: [] };
process_children(trimmed, { ...context, state: inner_state });

Expand All @@ -124,7 +121,9 @@ export function RegularElement(node, context) {
b.block([...state.init, ...build_template(inner_state.template)])
);

const statement = b.stmt(b.call('$$renderer.select', attributes, fn));
const [attributes, ...rest] = prepare_element_spread_object(node, context, optimiser.transform);

const statement = b.stmt(b.call('$$renderer.select', attributes, fn, ...rest));

if (optimiser.expressions.length > 0) {
context.state.template.push(
Expand All @@ -137,19 +136,7 @@ export function RegularElement(node, context) {
return;
}

if (node.name === 'option') {
const attributes = build_spread_object(
node,
node.attributes.filter(
(attribute) =>
attribute.type === 'Attribute' ||
attribute.type === 'BindDirective' ||
attribute.type === 'SpreadAttribute'
),
context,
optimiser.transform
);

if (is_option_special) {
let body;

if (node.metadata.synthetic_value_node) {
Expand All @@ -167,7 +154,9 @@ export function RegularElement(node, context) {
);
}

const statement = b.stmt(b.call('$$renderer.option', attributes, body));
const [attributes, ...rest] = prepare_element_spread_object(node, context, optimiser.transform);

const statement = b.stmt(b.call('$$renderer.option', attributes, body, ...rest));

if (optimiser.expressions.length > 0) {
context.state.template.push(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,42 +358,108 @@ function build_element_spread_attributes(
context,
transform
) {
const args = prepare_element_spread(
element,
/** @type {Array<AST.Attribute | AST.SpreadAttribute | AST.BindDirective>} */ (attributes),
style_directives,
class_directives,
context,
transform
);

let call = b.call('$.attributes', ...args);

context.state.template.push(call);
}

/**
* Prepare args for $.attributes(...): compute object, css_hash, classes, styles and flags.
* @param {AST.RegularElement | AST.SvelteElement} element
* @param {ComponentContext} context
* @param {(expression: Expression, metadata: ExpressionMetadata) => Expression} transform
* @returns {[ObjectExpression,Literal | undefined, ObjectExpression | undefined, ObjectExpression | undefined, Literal | undefined]}
*/
export function prepare_element_spread_object(element, context, transform) {
/** @type {Array<AST.Attribute | AST.SpreadAttribute | AST.BindDirective>} */
const select_attributes = [];
/** @type {AST.ClassDirective[]} */
const class_directives = [];
/** @type {AST.StyleDirective[]} */
const style_directives = [];

for (const attribute of element.attributes) {
if (
attribute.type === 'Attribute' ||
attribute.type === 'BindDirective' ||
attribute.type === 'SpreadAttribute'
) {
select_attributes.push(attribute);
} else if (attribute.type === 'ClassDirective') {
class_directives.push(attribute);
} else if (attribute.type === 'StyleDirective') {
style_directives.push(attribute);
}
}

return prepare_element_spread(
element,
select_attributes,
style_directives,
class_directives,
context,
transform
);
}

/**
* Prepare args for $.attributes(...): compute object, css_hash, classes, styles and flags.
* @param {AST.RegularElement | AST.SvelteElement} element
* @param {Array<AST.Attribute | AST.SpreadAttribute | AST.BindDirective>} attributes
* @param {AST.StyleDirective[]} style_directives
* @param {AST.ClassDirective[]} class_directives
* @param {ComponentContext} context
* @param {(expression: Expression, metadata: ExpressionMetadata) => Expression} transform
* @returns {[ObjectExpression,Literal | undefined, ObjectExpression | undefined, ObjectExpression | undefined, Literal | undefined]}
*/
export function prepare_element_spread(
element,
attributes,
style_directives,
class_directives,
context,
transform
) {
/** @type {ObjectExpression | undefined} */
let classes;
/** @type {ObjectExpression | undefined} */
let styles;
let flags = 0;

let has_await = false;

if (class_directives.length) {
const properties = class_directives.map((directive) => {
has_await ||= directive.metadata.expression.has_await;

return b.init(
const properties = class_directives.map((directive) =>
b.init(
directive.name,
directive.expression.type === 'Identifier' && directive.expression.name === directive.name
? b.id(directive.name)
: transform(
/** @type {Expression} */ (context.visit(directive.expression)),
directive.metadata.expression
)
);
});
)
);

classes = b.object(properties);
}

if (style_directives.length > 0) {
const properties = style_directives.map((directive) => {
has_await ||= directive.metadata.expression.has_await;

return b.init(
const properties = style_directives.map((directive) =>
b.init(
directive.name,
directive.value === true
? b.id(directive.name)
: build_attribute_value(directive.value, context, transform, true)
);
});

)
);
styles = b.object(properties);
}

Expand All @@ -406,17 +472,12 @@ function build_element_spread_attributes(
}

const object = build_spread_object(element, attributes, context, transform);

const css_hash =
element.metadata.scoped && context.state.analysis.css.hash
? b.literal(context.state.analysis.css.hash)
: undefined;

const args = [object, css_hash, classes, styles, flags ? b.literal(flags) : undefined];

let call = b.call('$.attributes', ...args);

context.state.template.push(call);
return [object, css_hash, classes, styles, flags ? b.literal(flags) : undefined];
}

/**
Expand Down
19 changes: 15 additions & 4 deletions packages/svelte/src/internal/server/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,16 @@ export class Renderer {
/**
* @param {Record<string, any>} attrs
* @param {(renderer: Renderer) => void} fn
* @param {string | undefined} [css_hash]
* @param {Record<string, boolean> | undefined} [classes]
* @param {Record<string, string> | undefined} [styles]
* @param {number | undefined} [flags]
* @returns {void}
*/
select({ value, ...attrs }, fn) {
this.push(`<select${attributes(attrs)}>`);
select(attrs, fn, css_hash, classes, styles, flags) {
const { value, ...select_attrs } = attrs;

this.push(`<select${attributes(select_attrs, css_hash, classes, styles, flags)}>`);
this.child((renderer) => {
renderer.local.select_value = value;
fn(renderer);
Expand All @@ -173,9 +180,13 @@ export class Renderer {
/**
* @param {Record<string, any>} attrs
* @param {string | number | boolean | ((renderer: Renderer) => void)} body
* @param {string | undefined} [css_hash]
* @param {Record<string, boolean> | undefined} [classes]
* @param {Record<string, string> | undefined} [styles]
* @param {number | undefined} [flags]
*/
option(attrs, body) {
this.#out.push(`<option${attributes(attrs)}`);
option(attrs, body, css_hash, classes, styles, flags) {
this.#out.push(`<option${attributes(attrs, css_hash, classes, styles, flags)}`);

/**
* @param {Renderer} renderer
Expand Down
18 changes: 18 additions & 0 deletions packages/svelte/src/internal/server/renderer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,24 @@ test('selects an option with an implicit value', () => {
);
});

test('select merges scoped css hash with static class', () => {
const component = (renderer: Renderer) => {
renderer.select(
{ class: 'foo', value: 'foo' },
(renderer) => {
renderer.option({ value: 'foo' }, (renderer) => renderer.push('foo'));
},
'svelte-hash'
);
};

const { head, body } = Renderer.render(component as unknown as Component);
expect(head).toBe('');
expect(body).toBe(
'<!--[--><select class="foo svelte-hash"><option value="foo" selected>foo</option></select><!--]-->'
);
});

describe('async', () => {
beforeAll(() => {
enable_async_mode_flag();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<select><option class="opt svelte-1l69ci" value="foo" selected>foo</option></select>

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<select value="foo">
<option class="opt" value="foo">foo</option>
</select>

<style>
option {
color: red;
}
</style>

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<select class="foo svelte-18avu97"><option value="foo" selected>foo</option></select>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<select class="foo" value="foo">
<option value="foo">foo</option>
</select>

<style>
select {
color: red;
}
</style>
Loading