Skip to content

Commit 3165950

Browse files
authored
fix: only create document.title effect if value is dynamic (#12698)
* fix: dont create an effect for static title * improve build_template_literal * tidy up * changeset * simplify * simplify * tweak
1 parent a1db493 commit 3165950

File tree

10 files changed

+65
-74
lines changed

10 files changed

+65
-74
lines changed

.changeset/strange-pears-perform.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: only create `document.title` effect if value is dynamic

packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,10 +209,11 @@ export function BindDirective(node, context) {
209209
)
210210
)?.value
211211
);
212+
212213
if (value !== undefined) {
213214
group_getter = b.thunk(
214215
b.block([
215-
b.stmt(build_attribute_value(value, context)[1]),
216+
b.stmt(build_attribute_value(value, context).value),
216217
b.return(/** @type {Expression} */ (context.visit(expression)))
217218
])
218219
);

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -229,17 +229,16 @@ export function RegularElement(node, context) {
229229
(attribute.value === true || is_text_attribute(attribute))
230230
) {
231231
const name = get_attribute_name(node, attribute, context);
232-
const literal_value = /** @type {Literal} */ (
233-
build_attribute_value(attribute.value, context)[1]
234-
).value;
235-
if (name !== 'class' || literal_value) {
232+
const value = is_text_attribute(attribute) ? attribute.value[0].data : true;
233+
234+
if (name !== 'class' || value) {
236235
// TODO namespace=foreign probably doesn't want to do template stuff at all and instead use programmatic methods
237236
// to create the elements it needs.
238237
context.state.template.push(
239238
` ${attribute.name}${
240-
is_boolean_attribute(name) && literal_value === true
239+
is_boolean_attribute(name) && value === true
241240
? ''
242-
: `="${literal_value === true ? '' : escape_html(literal_value, true)}"`
241+
: `="${value === true ? '' : escape_html(value, true)}"`
243242
}`
244243
);
245244
continue;
@@ -440,7 +439,7 @@ function build_element_spread_attributes(
440439
if (attribute.type === 'Attribute') {
441440
const name = get_attribute_name(element, attribute, context);
442441
// TODO: handle has_call
443-
const [, value] = build_attribute_value(attribute.value, context);
442+
const { value } = build_attribute_value(attribute.value, context);
444443

445444
if (
446445
name === 'is' &&
@@ -554,7 +553,7 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
554553
const name = get_attribute_name(element, attribute, context);
555554
const is_svg = context.state.metadata.namespace === 'svg' || element.name === 'svg';
556555
const is_mathml = context.state.metadata.namespace === 'mathml';
557-
let [has_call, value] = build_attribute_value(attribute.value, context);
556+
let { has_call, value } = build_attribute_value(attribute.value, context);
558557

559558
// The foreign namespace doesn't have any special handling, everything goes through the attr function
560559
if (context.state.metadata.namespace === 'foreign') {
@@ -636,7 +635,7 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
636635
function build_custom_element_attribute_update_assignment(node_id, attribute, context) {
637636
const state = context.state;
638637
const name = attribute.name; // don't lowercase, as we set the element's property, which might be case sensitive
639-
let [has_call, value] = build_attribute_value(attribute.value, context);
638+
let { has_call, value } = build_attribute_value(attribute.value, context);
640639

641640
const update = b.stmt(b.call('$.set_custom_element_data', node_id, b.literal(name), value));
642641

@@ -665,7 +664,7 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co
665664
*/
666665
function build_element_special_value_attribute(element, node_id, attribute, context) {
667666
const state = context.state;
668-
const [, value] = build_attribute_value(attribute.value, context);
667+
const { value } = build_attribute_value(attribute.value, context);
669668

670669
const inner_assignment = b.assignment(
671670
'=',
@@ -676,7 +675,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont
676675
value
677676
)
678677
);
679-
const is_reactive = attribute.metadata.expression.has_state;
678+
680679
const is_select_with_value =
681680
// attribute.metadata.dynamic would give false negatives because even if the value does not change,
682681
// the inner options could still change, so we need to always treat it as reactive
@@ -699,7 +698,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont
699698
state.init.push(b.stmt(b.call('$.init_select', node_id, b.thunk(value))));
700699
}
701700

702-
if (is_reactive) {
701+
if (attribute.metadata.expression.has_state) {
703702
const id = state.scope.generate(`${node_id.name}_value`);
704703
build_update_assignment(
705704
state,

packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ export function SlotElement(node, context) {
3030
if (attribute.type === 'SpreadAttribute') {
3131
spreads.push(b.thunk(/** @type {Expression} */ (context.visit(attribute))));
3232
} else if (attribute.type === 'Attribute') {
33-
const [, value] = build_attribute_value(attribute.value, context);
33+
const { value } = build_attribute_value(attribute.value, context);
34+
3435
if (attribute.name === 'name') {
3536
name = value;
3637
is_default = false;

packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export function SvelteElement(node, context) {
126126
get_tag,
127127
node.metadata.svg || node.metadata.mathml ? b.true : b.false,
128128
inner.length > 0 && b.arrow([element_id, b.id('$$anchor')], b.block(inner)),
129-
dynamic_namespace && b.thunk(build_attribute_value(dynamic_namespace, context)[1]),
129+
dynamic_namespace && b.thunk(build_attribute_value(dynamic_namespace, context).value),
130130
location && b.array([b.literal(location.line), b.literal(location.column)])
131131
)
132132
)
@@ -161,7 +161,7 @@ function build_dynamic_element_attributes(attributes, context, element_id) {
161161

162162
for (const attribute of attributes) {
163163
if (attribute.type === 'Attribute') {
164-
const [, value] = build_attribute_value(attribute.value, context);
164+
const { value } = build_attribute_value(attribute.value, context);
165165

166166
if (
167167
is_event_attribute(attribute) &&

packages/svelte/src/compiler/phases/3-transform/client/visitors/TitleElement.js

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,32 +8,17 @@ import { build_template_literal } from './shared/utils.js';
88
* @param {ComponentContext} context
99
*/
1010
export function TitleElement(node, context) {
11-
// TODO throw validation error when attributes present / when children something else than text/expression tags
12-
// TODO only create update when expression is dynamic
11+
const { has_state, value } = build_template_literal(
12+
/** @type {any} */ (node.fragment.nodes),
13+
context.visit,
14+
context.state
15+
);
1316

14-
if (node.fragment.nodes.length === 1 && node.fragment.nodes[0].type === 'Text') {
15-
context.state.init.push(
16-
b.stmt(
17-
b.assignment(
18-
'=',
19-
b.member(b.id('$.document'), b.id('title')),
20-
b.literal(/** @type {Text} */ (node.fragment.nodes[0]).data)
21-
)
22-
)
23-
);
17+
const statement = b.stmt(b.assignment('=', b.member(b.id('$.document'), b.id('title')), value));
18+
19+
if (has_state) {
20+
context.state.update.push(statement);
2421
} else {
25-
context.state.update.push(
26-
b.stmt(
27-
b.assignment(
28-
'=',
29-
b.member(b.id('$.document'), b.id('title')),
30-
build_template_literal(
31-
/** @type {any} */ (node.fragment.nodes),
32-
context.visit,
33-
context.state
34-
)[1]
35-
)
36-
)
37-
);
22+
context.state.init.push(statement);
3823
}
3924
}

packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export function build_component(node, component_name, context, anchor = context.
9393
} else if (attribute.type === 'Attribute') {
9494
if (attribute.name.startsWith('--')) {
9595
custom_css_props.push(
96-
b.init(attribute.name, build_attribute_value(attribute.value, context)[1])
96+
b.init(attribute.name, build_attribute_value(attribute.value, context).value)
9797
);
9898
continue;
9999
}
@@ -106,7 +106,7 @@ export function build_component(node, component_name, context, anchor = context.
106106
has_children_prop = true;
107107
}
108108

109-
const [, value] = build_attribute_value(attribute.value, context);
109+
const { value } = build_attribute_value(attribute.value, context);
110110

111111
if (attribute.metadata.expression.has_state) {
112112
let arg = value;

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export function build_style_directives(
3333
let value =
3434
directive.value === true
3535
? build_getter({ name: directive.name, type: 'Identifier' }, context.state)
36-
: build_attribute_value(directive.value, context)[1];
36+
: build_attribute_value(directive.value, context).value;
3737

3838
const update = b.stmt(
3939
b.call(
@@ -92,24 +92,24 @@ export function build_class_directives(
9292
/**
9393
* @param {Attribute['value']} value
9494
* @param {ComponentContext} context
95-
* @returns {[has_call: boolean, Expression]}
9695
*/
9796
export function build_attribute_value(value, context) {
9897
if (value === true) {
99-
return [false, b.literal(true)];
98+
return { has_state: false, has_call: false, value: b.literal(true) };
10099
}
101100

102101
if (!Array.isArray(value) || value.length === 1) {
103102
const chunk = Array.isArray(value) ? value[0] : value;
104103

105104
if (chunk.type === 'Text') {
106-
return [false, b.literal(chunk.data)];
105+
return { has_state: false, has_call: false, value: b.literal(chunk.data) };
107106
}
108107

109-
return [
110-
chunk.metadata.expression.has_call,
111-
/** @type {Expression} */ (context.visit(chunk.expression))
112-
];
108+
return {
109+
has_state: chunk.metadata.expression.has_call,
110+
has_call: chunk.metadata.expression.has_call,
111+
value: /** @type {Expression} */ (context.visit(chunk.expression))
112+
};
113113
}
114114

115115
return build_template_literal(value, context.visit, context.state);

packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,18 +66,13 @@ export function process_children(nodes, expression, is_element, { visit, state }
6666

6767
state.template.push(' ');
6868

69-
const [has_call, value] = build_template_literal(sequence, visit, state);
69+
const { has_state, has_call, value } = build_template_literal(sequence, visit, state);
7070

7171
const update = b.stmt(b.call('$.set_text', text_id, value));
7272

7373
if (has_call && !within_bound_contenteditable) {
7474
state.init.push(build_update(update));
75-
} else if (
76-
sequence.some(
77-
(node) => node.type === 'ExpressionTag' && node.metadata.expression.has_state
78-
) &&
79-
!within_bound_contenteditable
80-
) {
75+
} else if (has_state && !within_bound_contenteditable) {
8176
state.update.push(update);
8277
} else {
8378
state.init.push(b.stmt(b.assignment('=', b.member(text_id, b.id('nodeValue')), value)));

packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,39 +14,41 @@ import { locator } from '../../../../../state.js';
1414
* @param {Array<Text | ExpressionTag>} values
1515
* @param {(node: SvelteNode, state: any) => any} visit
1616
* @param {ComponentClientTransformState} state
17-
* @returns {[boolean, TemplateLiteral]}
1817
*/
1918
export function build_template_literal(values, visit, state) {
20-
/** @type {TemplateElement[]} */
21-
const quasis = [];
22-
2319
/** @type {Expression[]} */
2420
const expressions = [];
21+
22+
let quasi = b.quasi('');
23+
const quasis = [quasi];
24+
2525
let has_call = false;
26+
let has_state = false;
2627
let contains_multiple_call_expression = false;
27-
quasis.push(b.quasi(''));
2828

2929
for (let i = 0; i < values.length; i++) {
3030
const node = values[i];
3131

32-
if (node.type === 'ExpressionTag' && node.metadata.expression.has_call) {
33-
if (has_call) {
34-
contains_multiple_call_expression = true;
32+
if (node.type === 'ExpressionTag') {
33+
if (node.metadata.expression.has_call) {
34+
if (has_call) {
35+
contains_multiple_call_expression = true;
36+
}
37+
has_call = true;
3538
}
36-
has_call = true;
39+
40+
has_state ||= node.metadata.expression.has_state;
3741
}
3842
}
3943

4044
for (let i = 0; i < values.length; i++) {
4145
const node = values[i];
4246

4347
if (node.type === 'Text') {
44-
const last = /** @type {TemplateElement} */ (quasis.at(-1));
45-
last.value.raw += sanitize_template_string(node.data);
48+
quasi.value.raw += sanitize_template_string(node.data);
4649
} else if (node.type === 'ExpressionTag' && node.expression.type === 'Literal') {
47-
const last = /** @type {TemplateElement} */ (quasis.at(-1));
4850
if (node.expression.value != null) {
49-
last.value.raw += sanitize_template_string(node.expression.value + '');
51+
quasi.value.raw += sanitize_template_string(node.expression.value + '');
5052
}
5153
} else {
5254
if (contains_multiple_call_expression) {
@@ -65,12 +67,15 @@ export function build_template_literal(values, visit, state) {
6567
} else {
6668
expressions.push(b.logical('??', visit(node.expression, state), b.literal('')));
6769
}
68-
quasis.push(b.quasi('', i + 1 === values.length));
70+
71+
quasi = b.quasi('', i + 1 === values.length);
72+
quasis.push(quasi);
6973
}
7074
}
7175

72-
// TODO instead of this tuple, return a `{ dynamic, complex, value }` object. will DRY stuff out
73-
return [has_call, b.template(quasis, expressions)];
76+
const value = b.template(quasis, expressions);
77+
78+
return { value, has_state, has_call };
7479
}
7580

7681
/**

0 commit comments

Comments
 (0)