diff --git a/.changeset/tasty-pigs-greet.md b/.changeset/tasty-pigs-greet.md new file mode 100644 index 0000000000..63decceed8 --- /dev/null +++ b/.changeset/tasty-pigs-greet.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: avoid double deriveds in component props diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index c59a5544df..421118cf68 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -269,41 +269,6 @@ export function should_proxy(node, scope) { return true; } -/** - * @param {Pattern} node - * @param {import('zimmerframe').Context} context - * @returns {{ id: Pattern, declarations: null | Statement[] }} - */ -export function create_derived_block_argument(node, context) { - if (node.type === 'Identifier') { - context.state.transform[node.name] = { read: get_value }; - return { id: node, declarations: null }; - } - - const pattern = /** @type {Pattern} */ (context.visit(node)); - const identifiers = extract_identifiers(node); - - const id = b.id('$$source'); - const value = b.id('$$value'); - - const block = b.block([ - b.var(pattern, b.call('$.get', id)), - b.return(b.object(identifiers.map((identifier) => b.prop('init', identifier, identifier)))) - ]); - - const declarations = [b.var(value, create_derived(context.state, b.thunk(block)))]; - - for (const id of identifiers) { - context.state.transform[id.name] = { read: get_value }; - - declarations.push( - b.var(id, create_derived(context.state, b.thunk(b.member(b.call('$.get', value), id)))) - ); - } - - return { id, declarations }; -} - /** * Svelte legacy mode should use safe equals in most places, runes mode shouldn't * @param {ComponentClientTransformState} state diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js index 146f75d405..e0aef2d316 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js @@ -1,8 +1,10 @@ /** @import { BlockStatement, Expression, Pattern, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ -/** @import { ComponentContext } from '../types' */ +/** @import { ComponentClientTransformState, ComponentContext } from '../types' */ +import { extract_identifiers } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; -import { create_derived_block_argument } from '../utils.js'; +import { create_derived } from '../utils.js'; +import { get_value } from './shared/declarations.js'; /** * @param {AST.AwaitBlock} node @@ -65,3 +67,38 @@ export function AwaitBlock(node, context) { ) ); } + +/** + * @param {Pattern} node + * @param {import('zimmerframe').Context} context + * @returns {{ id: Pattern, declarations: null | Statement[] }} + */ +function create_derived_block_argument(node, context) { + if (node.type === 'Identifier') { + context.state.transform[node.name] = { read: get_value }; + return { id: node, declarations: null }; + } + + const pattern = /** @type {Pattern} */ (context.visit(node)); + const identifiers = extract_identifiers(node); + + const id = b.id('$$source'); + const value = b.id('$$value'); + + const block = b.block([ + b.var(pattern, b.call('$.get', id)), + b.return(b.object(identifiers.map((identifier) => b.prop('init', identifier, identifier)))) + ]); + + const declarations = [b.var(value, create_derived(context.state, b.thunk(block)))]; + + for (const id of identifiers) { + context.state.transform[id.name] = { read: get_value }; + + declarations.push( + b.var(id, create_derived(context.state, b.thunk(b.member(b.call('$.get', value), id)))) + ); + } + + return { id, declarations }; +} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 21a78de032..d0a148f216 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -537,8 +537,8 @@ function build_element_attribute_update_assignment( const is_svg = context.state.metadata.namespace === 'svg' || element.name === 'svg'; const is_mathml = context.state.metadata.namespace === 'mathml'; - let { value, has_state } = build_attribute_value(attribute.value, context, (value) => - get_expression_id(state, value) + let { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) => + metadata.has_call ? get_expression_id(state, value) : value ); if (name === 'autofocus') { @@ -665,8 +665,8 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co */ function build_element_special_value_attribute(element, node_id, attribute, context) { const state = context.state; - const { value, has_state } = build_attribute_value(attribute.value, context, (value) => - get_expression_id(state, value) + const { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) => + metadata.has_call ? get_expression_id(state, value) : value ); const inner_assignment = b.assignment( diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js index f1b08acbc6..fdd705e32e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js @@ -30,8 +30,10 @@ export function SlotElement(node, context) { if (attribute.type === 'SpreadAttribute') { spreads.push(b.thunk(/** @type {Expression} */ (context.visit(attribute)))); } else if (attribute.type === 'Attribute') { - const { value, has_state } = build_attribute_value(attribute.value, context, (value) => - memoize_expression(context.state, value) + const { value, has_state } = build_attribute_value( + attribute.value, + context, + (value, metadata) => (metadata.has_call ? memoize_expression(context.state, value) : value) ); if (attribute.name === 'name') { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index 9ac0bac120..15e4f68e9e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -4,7 +4,6 @@ import { dev, is_ignored } from '../../../../../state.js'; import { get_attribute_chunks, object } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; -import { create_derived } from '../../utils.js'; import { build_bind_this, memoize_expression, validate_binding } from '../shared/utils.js'; import { build_attribute_value } from '../shared/element.js'; import { build_event_handler } from './events.js'; @@ -134,9 +133,9 @@ export function build_component(node, component_name, context, anchor = context. custom_css_props.push( b.init( attribute.name, - build_attribute_value(attribute.value, context, (value) => + build_attribute_value(attribute.value, context, (value, metadata) => // TODO put the derived in the local block - memoize_expression(context.state, value) + metadata.has_call ? memoize_expression(context.state, value) : value ).value ) ); @@ -151,31 +150,29 @@ export function build_component(node, component_name, context, anchor = context. has_children_prop = true; } - const { value, has_state } = build_attribute_value(attribute.value, context, (value) => - memoize_expression(context.state, value) - ); - - if (has_state) { - let arg = value; - - // When we have a non-simple computation, anything other than an Identifier or Member expression, - // then there's a good chance it needs to be memoized to avoid over-firing when read within the - // child component. - const should_wrap_in_derived = get_attribute_chunks(attribute.value).some((n) => { - return ( - n.type === 'ExpressionTag' && - n.expression.type !== 'Identifier' && - n.expression.type !== 'MemberExpression' - ); - }); + const { value, has_state } = build_attribute_value( + attribute.value, + context, + (value, metadata) => { + if (!metadata.has_state) return value; + + // When we have a non-simple computation, anything other than an Identifier or Member expression, + // then there's a good chance it needs to be memoized to avoid over-firing when read within the + // child component (e.g. `active={i === index}`) + const should_wrap_in_derived = get_attribute_chunks(attribute.value).some((n) => { + return ( + n.type === 'ExpressionTag' && + n.expression.type !== 'Identifier' && + n.expression.type !== 'MemberExpression' + ); + }); - if (should_wrap_in_derived) { - const id = b.id(context.state.scope.generate(attribute.name)); - context.state.init.push(b.var(id, create_derived(context.state, b.thunk(value)))); - arg = b.call('$.get', id); + return should_wrap_in_derived ? memoize_expression(context.state, value) : value; } + ); - push_prop(b.get(attribute.name, [b.return(arg)])); + if (has_state) { + push_prop(b.get(attribute.name, [b.return(value)])); } else { push_prop(b.init(attribute.name, value)); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js index 8fb6b8bdde..abffad0ff7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js @@ -1,11 +1,11 @@ /** @import { Expression, Identifier, ObjectExpression } from 'estree' */ -/** @import { AST } from '#compiler' */ +/** @import { AST, ExpressionMetadata } from '#compiler' */ /** @import { ComponentClientTransformState, ComponentContext } from '../../types' */ import { normalize_attribute } from '../../../../../../utils.js'; import { is_ignored } from '../../../../../state.js'; import { is_event_attribute } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; -import { build_getter, create_derived } from '../../utils.js'; +import { build_getter } from '../../utils.js'; import { build_template_chunk, get_expression_id } from './utils.js'; /** @@ -35,8 +35,10 @@ export function build_set_attributes( for (const attribute of attributes) { if (attribute.type === 'Attribute') { - const { value, has_state } = build_attribute_value(attribute.value, context, (value) => - get_expression_id(context.state, value) + const { value, has_state } = build_attribute_value( + attribute.value, + context, + (value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value) ); if ( @@ -59,10 +61,9 @@ export function build_set_attributes( let value = /** @type {Expression} */ (context.visit(attribute)); if (attribute.metadata.expression.has_call) { - const id = b.id(state.scope.generate('spread_with_call')); - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); + value = get_expression_id(context.state, value); } + values.push(b.spread(value)); } } @@ -111,8 +112,8 @@ export function build_style_directives( let value = directive.value === true ? build_getter({ name: directive.name, type: 'Identifier' }, context.state) - : build_attribute_value(directive.value, context, (value) => - get_expression_id(context.state, value) + : build_attribute_value(directive.value, context, (value, metadata) => + metadata.has_call ? get_expression_id(context.state, value) : value ).value; const update = b.stmt( @@ -169,7 +170,7 @@ export function build_class_directives( /** * @param {AST.Attribute['value']} value * @param {ComponentContext} context - * @param {(value: Expression) => Expression} memoize + * @param {(value: Expression, metadata: ExpressionMetadata) => Expression} memoize * @returns {{ value: Expression, has_state: boolean }} */ export function build_attribute_value(value, context, memoize = (value) => value) { @@ -187,7 +188,7 @@ export function build_attribute_value(value, context, memoize = (value) => value let expression = /** @type {Expression} */ (context.visit(chunk.expression)); return { - value: chunk.metadata.expression.has_call ? memoize(expression) : expression, + value: memoize(expression, chunk.metadata.expression), has_state: chunk.metadata.expression.has_state }; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index c4f81274d9..4c28616665 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js @@ -1,5 +1,5 @@ /** @import { Expression, ExpressionStatement, Identifier, MemberExpression, SequenceExpression, Statement, Super } from 'estree' */ -/** @import { AST } from '#compiler' */ +/** @import { AST, ExpressionMetadata } from '#compiler' */ /** @import { ComponentClientTransformState } from '../../types' */ import { walk } from 'zimmerframe'; import { object } from '../../../../../utils/ast.js'; @@ -79,14 +79,14 @@ function compare_expressions(a, b) { * @param {Array} values * @param {(node: AST.SvelteNode, state: any) => any} visit * @param {ComponentClientTransformState} state - * @param {(value: Expression) => Expression} memoize + * @param {(value: Expression, metadata: ExpressionMetadata) => Expression} memoize * @returns {{ value: Expression, has_state: boolean }} */ export function build_template_chunk( values, visit, state, - memoize = (value) => get_expression_id(state, value) + memoize = (value, metadata) => (metadata.has_call ? get_expression_id(state, value) : value) ) { /** @type {Expression[]} */ const expressions = []; @@ -106,14 +106,13 @@ export function build_template_chunk( quasi.value.cooked += node.expression.value + ''; } } else { - let value = /** @type {Expression} */ (visit(node.expression, state)); + let value = memoize( + /** @type {Expression} */ (visit(node.expression, state)), + node.metadata.expression + ); has_state ||= node.metadata.expression.has_state; - if (node.metadata.expression.has_call) { - value = memoize(value); - } - if (values.length === 1) { // If we have a single expression, then pass that in directly to possibly avoid doing // extra work in the template_effect (instead we do the work in set_text).