msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

fix: SSR scoped classes for <select value> elements (#16821)

Browse Source 
* fix: preserve scoped classes on select during ssr

* test: cover select scoped class regression

* chore: changeset

* chore: format files after lint

* fix: unify <select> attribute handling and prevent double-await

* test: update renderer.select call order

* fix: restore scoped classes on <option>

* test: cover scoped class for <option>

* dry

* de-diff

* tweak changeset

---------

Co-authored-by: 7nik <kifiranet@gmail.com>
Co-authored-by: Rich Harris <rich.harris@vercel.com>
pull/16824/head
LeeWxx 17 hours ago committed by GitHub
parent 24944e61f5
commit ac7e160029
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 172 additions and 66 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/nasty-comics-play.md
Unescape View File

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: SSR regression of processing attributes of `<select>` and `<option>`

67
packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js
Unescape View File

@ -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';
@ -37,9 +36,27 @@ export function RegularElement(node, context) {
const optimiser = new PromiseOptimiser();
// 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}`));
const body = build_element_attributes(node, { ...context, state }, optimiser.transform);
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(
@ -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 });
@ -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(
@ -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) {
@ -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(

99
packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js
Unescape View File

@ -358,17 +358,86 @@ 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)
@ -376,24 +445,21 @@ function build_element_spread_attributes(
/** @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);
}
@ -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];
}
/**

19
packages/svelte/src/internal/server/renderer.js
Unescape View File

@ -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);
@ -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

18
packages/svelte/src/internal/server/renderer.test.ts
Unescape View File

@ -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();

2
packages/svelte/tests/server-side-rendering/samples/option-scoped-class/_expected.html
Unescape View File

@ -0,0 +1,2 @@
<select><option class="opt svelte-1l69ci" value="foo" selected>foo</option></select>

10
packages/svelte/tests/server-side-rendering/samples/option-scoped-class/main.svelte
Unescape View File

@ -0,0 +1,10 @@
<select value="foo">
<option class="opt" value="foo">foo</option>
</select>
<style>
option {
color: red;
}
</style>

1
packages/svelte/tests/server-side-rendering/samples/select-value-scoped-class/_expected.html
Unescape View File

@ -0,0 +1 @@
<select class="foo svelte-18avu97"><option value="foo" selected>foo</option></select>

9
packages/svelte/tests/server-side-rendering/samples/select-value-scoped-class/main.svelte
Unescape View File

@ -0,0 +1,9 @@
<select class="foo" value="foo">
<option value="foo">foo</option>
</select>
<style>
select {
color: red;
}
</style>
Write Preview
Loading…
Cancel
Save