Skip to content

Commit 0488ae8

Browse files
Rich-HarrisRich-Harris
committed
fix: prevent double deriveds in component props
1 parent c75f1f5 commit 0488ae8

File tree

5 files changed

+46
-45
lines changed

5 files changed

+46
-45
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -537,8 +537,8 @@ function build_element_attribute_update_assignment(
537537
const is_svg = context.state.metadata.namespace === 'svg' || element.name === 'svg';
538538
const is_mathml = context.state.metadata.namespace === 'mathml';
539539

540-
let { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
541-
get_expression_id(state, value)
540+
let { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) =>
541+
metadata.has_call ? get_expression_id(state, value) : value
542542
);
543543

544544
if (name === 'autofocus') {
@@ -665,8 +665,8 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co
665665
*/
666666
function build_element_special_value_attribute(element, node_id, attribute, context) {
667667
const state = context.state;
668-
const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
669-
get_expression_id(state, value)
668+
const { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) =>
669+
metadata.has_call ? get_expression_id(state, value) : value
670670
);
671671

672672
const inner_assignment = b.assignment(

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ 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, has_state } = build_attribute_value(attribute.value, context, (value) =>
34-
memoize_expression(context.state, value)
33+
const { value, has_state } = build_attribute_value(
34+
attribute.value,
35+
context,
36+
(value, metadata) => (metadata.has_call ? memoize_expression(context.state, value) : value)
3537
);
3638

3739
if (attribute.name === 'name') {

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

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,9 @@ export function build_component(node, component_name, context, anchor = context.
134134
custom_css_props.push(
135135
b.init(
136136
attribute.name,
137-
build_attribute_value(attribute.value, context, (value) =>
137+
build_attribute_value(attribute.value, context, (value, metadata) =>
138138
// TODO put the derived in the local block
139-
memoize_expression(context.state, value)
139+
metadata.has_call ? memoize_expression(context.state, value) : value
140140
).value
141141
)
142142
);
@@ -151,31 +151,29 @@ export function build_component(node, component_name, context, anchor = context.
151151
has_children_prop = true;
152152
}
153153

154-
const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
155-
memoize_expression(context.state, value)
156-
);
157-
158-
if (has_state) {
159-
let arg = value;
160-
161-
// When we have a non-simple computation, anything other than an Identifier or Member expression,
162-
// then there's a good chance it needs to be memoized to avoid over-firing when read within the
163-
// child component.
164-
const should_wrap_in_derived = get_attribute_chunks(attribute.value).some((n) => {
165-
return (
166-
n.type === 'ExpressionTag' &&
167-
n.expression.type !== 'Identifier' &&
168-
n.expression.type !== 'MemberExpression'
169-
);
170-
});
154+
const { value, has_state } = build_attribute_value(
155+
attribute.value,
156+
context,
157+
(value, metadata) => {
158+
if (!metadata.has_state) return value;
159+
160+
// When we have a non-simple computation, anything other than an Identifier or Member expression,
161+
// then there's a good chance it needs to be memoized to avoid over-firing when read within the
162+
// child component (e.g. `active={i === index}`)
163+
const should_wrap_in_derived = get_attribute_chunks(attribute.value).some((n) => {
164+
return (
165+
n.type === 'ExpressionTag' &&
166+
n.expression.type !== 'Identifier' &&
167+
n.expression.type !== 'MemberExpression'
168+
);
169+
});
171170

172-
if (should_wrap_in_derived) {
173-
const id = b.id(context.state.scope.generate(attribute.name));
174-
context.state.init.push(b.var(id, create_derived(context.state, b.thunk(value))));
175-
arg = b.call('$.get', id);
171+
return should_wrap_in_derived ? memoize_expression(context.state, value) : value;
176172
}
173+
);
177174

178-
push_prop(b.get(attribute.name, [b.return(arg)]));
175+
if (has_state) {
176+
push_prop(b.get(attribute.name, [b.return(value)]));
179177
} else {
180178
push_prop(b.init(attribute.name, value));
181179
}

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/** @import { Expression, Identifier, ObjectExpression } from 'estree' */
2-
/** @import { AST } from '#compiler' */
2+
/** @import { AST, ExpressionMetadata } from '#compiler' */
33
/** @import { ComponentClientTransformState, ComponentContext } from '../../types' */
44
import { normalize_attribute } from '../../../../../../utils.js';
55
import { is_ignored } from '../../../../../state.js';
@@ -35,8 +35,10 @@ export function build_set_attributes(
3535

3636
for (const attribute of attributes) {
3737
if (attribute.type === 'Attribute') {
38-
const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
39-
get_expression_id(context.state, value)
38+
const { value, has_state } = build_attribute_value(
39+
attribute.value,
40+
context,
41+
(value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value)
4042
);
4143

4244
if (
@@ -111,8 +113,8 @@ export function build_style_directives(
111113
let value =
112114
directive.value === true
113115
? build_getter({ name: directive.name, type: 'Identifier' }, context.state)
114-
: build_attribute_value(directive.value, context, (value) =>
115-
get_expression_id(context.state, value)
116+
: build_attribute_value(directive.value, context, (value, metadata) =>
117+
metadata.has_call ? get_expression_id(context.state, value) : value
116118
).value;
117119

118120
const update = b.stmt(
@@ -169,7 +171,7 @@ export function build_class_directives(
169171
/**
170172
* @param {AST.Attribute['value']} value
171173
* @param {ComponentContext} context
172-
* @param {(value: Expression) => Expression} memoize
174+
* @param {(value: Expression, metadata: ExpressionMetadata) => Expression} memoize
173175
* @returns {{ value: Expression, has_state: boolean }}
174176
*/
175177
export function build_attribute_value(value, context, memoize = (value) => value) {
@@ -187,7 +189,7 @@ export function build_attribute_value(value, context, memoize = (value) => value
187189
let expression = /** @type {Expression} */ (context.visit(chunk.expression));
188190

189191
return {
190-
value: chunk.metadata.expression.has_call ? memoize(expression) : expression,
192+
value: memoize(expression, chunk.metadata.expression),
191193
has_state: chunk.metadata.expression.has_state
192194
};
193195
}

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, SequenceExpression, Statement, Super } from 'estree' */
2-
/** @import { AST } from '#compiler' */
2+
/** @import { AST, ExpressionMetadata } from '#compiler' */
33
/** @import { ComponentClientTransformState } from '../../types' */
44
import { walk } from 'zimmerframe';
55
import { object } from '../../../../../utils/ast.js';
@@ -79,14 +79,14 @@ function compare_expressions(a, b) {
7979
* @param {Array<AST.Text | AST.ExpressionTag>} values
8080
* @param {(node: AST.SvelteNode, state: any) => any} visit
8181
* @param {ComponentClientTransformState} state
82-
* @param {(value: Expression) => Expression} memoize
82+
* @param {(value: Expression, metadata: ExpressionMetadata) => Expression} memoize
8383
* @returns {{ value: Expression, has_state: boolean }}
8484
*/
8585
export function build_template_chunk(
8686
values,
8787
visit,
8888
state,
89-
memoize = (value) => get_expression_id(state, value)
89+
memoize = (value, metadata) => (metadata.has_call ? get_expression_id(state, value) : value)
9090
) {
9191
/** @type {Expression[]} */
9292
const expressions = [];
@@ -106,14 +106,13 @@ export function build_template_chunk(
106106
quasi.value.cooked += node.expression.value + '';
107107
}
108108
} else {
109-
let value = /** @type {Expression} */ (visit(node.expression, state));
109+
let value = memoize(
110+
/** @type {Expression} */ (visit(node.expression, state)),
111+
node.metadata.expression
112+
);
110113

111114
has_state ||= node.metadata.expression.has_state;
112115

113-
if (node.metadata.expression.has_call) {
114-
value = memoize(value);
115-
}
116-
117116
if (values.length === 1) {
118117
// If we have a single expression, then pass that in directly to possibly avoid doing
119118
// extra work in the template_effect (instead we do the work in set_text).

0 commit comments

Comments
 (0)