diff --git a/.changeset/calm-mice-perform.md b/.changeset/calm-mice-perform.md new file mode 100644 index 0000000000..f43c7fcf0e --- /dev/null +++ b/.changeset/calm-mice-perform.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: remove template expression inlining diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js index ca0ea55d1f..6c050d966a 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js @@ -1,7 +1,7 @@ /** @import { ArrowFunctionExpression, Expression, FunctionDeclaration, FunctionExpression } from 'estree' */ /** @import { AST, DelegatedEvent, SvelteNode } from '#compiler' */ /** @import { Context } from '../types' */ -import { is_boolean_attribute, is_capture_event, is_delegated } from '../../../../utils.js'; +import { cannot_be_set_statically, is_capture_event, is_delegated } from '../../../../utils.js'; import { get_attribute_chunks, get_attribute_expression, @@ -30,12 +30,12 @@ export function Attribute(node, context) { } } - if (node.name.startsWith('on')) { + if (is_event_attribute(node)) { mark_subtree_dynamic(context.path); } - if (parent.type === 'RegularElement' && is_boolean_attribute(node.name.toLowerCase())) { - node.metadata.expression.can_inline = false; + if (cannot_be_set_statically(node.name)) { + mark_subtree_dynamic(context.path); } if (node.value !== true) { @@ -51,7 +51,6 @@ export function Attribute(node, context) { node.metadata.expression.has_state ||= chunk.metadata.expression.has_state; node.metadata.expression.has_call ||= chunk.metadata.expression.has_call; - node.metadata.expression.can_inline &&= chunk.metadata.expression.can_inline; } if (is_event_attribute(node)) { diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js index fe3f638a69..c7ade4856b 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -179,8 +179,6 @@ export function CallExpression(node, context) { if (!is_pure(node.callee, context) || context.state.expression.dependencies.size > 0) { context.state.expression.has_call = true; context.state.expression.has_state = true; - context.state.expression.can_inline = false; - mark_subtree_dynamic(context.path); } } } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js index f59b7fc569..32c8d2ca36 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js @@ -2,6 +2,7 @@ /** @import { Context } from '../types' */ import { is_tag_valid_with_parent } from '../../../../html-tree-validation.js'; import * as e from '../../../errors.js'; +import { mark_subtree_dynamic } from './shared/fragment.js'; /** * @param {AST.ExpressionTag} node @@ -14,5 +15,9 @@ export function ExpressionTag(node, context) { } } + // TODO ideally we wouldn't do this here, we'd just do it on encountering + // an `Identifier` within the tag. But we currently need to handle `{42}` etc + mark_subtree_dynamic(context.path); + context.next({ ...context.state, expression: node.metadata.expression }); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js index 042c467df1..79dccd5a7c 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js @@ -1,4 +1,5 @@ /** @import { Expression, Identifier } from 'estree' */ +/** @import { EachBlock } from '#compiler' */ /** @import { Context } from '../types' */ import is_reference from 'is-reference'; import { should_proxy } from '../../3-transform/client/utils.js'; @@ -19,6 +20,8 @@ export function Identifier(node, context) { return; } + mark_subtree_dynamic(context.path); + // If we are using arguments outside of a function, then throw an error if ( node.name === 'arguments' && @@ -84,12 +87,6 @@ export function Identifier(node, context) { } } - // no binding means global, and we can't inline e.g. `{location}` - // because it could change between component renders. if there _is_ a - // binding and it is outside module scope, the expression cannot - // be inlined (TODO allow inlining in more cases - e.g. primitive consts) - let can_inline = !!binding && !binding.scope.parent && binding.kind === 'normal'; - if (binding) { if (context.state.expression) { context.state.expression.dependencies.add(binding); @@ -125,12 +122,4 @@ export function Identifier(node, context) { w.reactive_declaration_module_script_dependency(node); } } - - if (!can_inline) { - if (context.state.expression) { - context.state.expression.can_inline = false; - } - - mark_subtree_dynamic(context.path); - } } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js index 88adecbd35..1cc20c96da 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js @@ -20,9 +20,6 @@ export function MemberExpression(node, context) { if (context.state.expression && !is_pure(node, context)) { context.state.expression.has_state = true; - context.state.expression.can_inline = false; - - mark_subtree_dynamic(context.path); } if (!is_safe_identifier(node, context.state.scope)) { diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js index d5964f9ae1..7d1c4aaeaa 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js @@ -1,6 +1,6 @@ /** @import { AST } from '#compiler' */ /** @import { Context } from '../types' */ -import { cannot_be_set_statically, is_mathml, is_svg, is_void } from '../../../../utils.js'; +import { is_mathml, is_svg, is_void } from '../../../../utils.js'; import { is_tag_valid_with_ancestor, is_tag_valid_with_parent @@ -75,14 +75,6 @@ export function RegularElement(node, context) { node.attributes.push(create_attribute('value', child.start, child.end, [child])); } - if ( - node.attributes.some( - (attribute) => attribute.type === 'Attribute' && cannot_be_set_statically(attribute.name) - ) - ) { - mark_subtree_dynamic(context.path); - } - const binding = context.state.scope.get(node.name); if ( binding !== null && diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/TaggedTemplateExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/TaggedTemplateExpression.js index 724b9af311..eacb8a342a 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/TaggedTemplateExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/TaggedTemplateExpression.js @@ -10,7 +10,6 @@ export function TaggedTemplateExpression(node, context) { if (context.state.expression && !is_pure(node.tag, context)) { context.state.expression.has_call = true; context.state.expression.has_state = true; - context.state.expression.can_inline = false; } if (node.tag.type === 'Identifier') { 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 910f173f79..0b49d18ee9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -3,16 +3,16 @@ /** @import { ClientTransformState, ComponentClientTransformState, ComponentContext } from './types.js' */ /** @import { Analysis } from '../../types.js' */ /** @import { Scope } from '../../scope.js' */ +import * as b from '../../../utils/builders.js'; +import { extract_identifiers, is_simple_expression } from '../../../utils/ast.js'; import { - PROPS_IS_BINDABLE, - PROPS_IS_IMMUTABLE, PROPS_IS_LAZY_INITIAL, + PROPS_IS_IMMUTABLE, PROPS_IS_RUNES, - PROPS_IS_UPDATED + PROPS_IS_UPDATED, + PROPS_IS_BINDABLE } from '../../../../constants.js'; import { dev } from '../../../state.js'; -import { extract_identifiers, is_simple_expression } from '../../../utils/ast.js'; -import * as b from '../../../utils/builders.js'; import { get_value } from './visitors/shared/declarations.js'; /** diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js index 111516ce0a..0e6ea29614 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js @@ -141,14 +141,14 @@ export function Fragment(node, context) { const id = b.id(context.state.scope.generate('fragment')); const use_space_template = - trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag') && - trimmed.some((node) => node.type === 'ExpressionTag' && !node.metadata.expression.can_inline); + trimmed.some((node) => node.type === 'ExpressionTag') && + trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag'); if (use_space_template) { // special case — we can use `$.text` instead of creating a unique template const id = b.id(context.state.scope.generate('text')); - process_children(trimmed, () => id, null, { + process_children(trimmed, () => id, false, { ...context, state }); @@ -158,12 +158,12 @@ export function Fragment(node, context) { } else { if (is_standalone) { // no need to create a template, we can just use the existing block's anchor - process_children(trimmed, () => b.id('$$anchor'), null, { ...context, state }); + process_children(trimmed, () => b.id('$$anchor'), false, { ...context, state }); } else { /** @type {(is_text: boolean) => Expression} */ const expression = (is_text) => b.call('$.first_child', id, is_text && b.true); - process_children(trimmed, expression, null, { ...context, state }); + process_children(trimmed, expression, false, { ...context, state }); let flags = TEMPLATE_FRAGMENT; 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 19948464d2..85df92e8bf 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 @@ -1,9 +1,8 @@ -/** @import { Expression, ExpressionStatement, Identifier, Literal, MemberExpression, ObjectExpression, Statement } from 'estree' */ +/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, ObjectExpression, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { SourceLocation } from '#shared' */ /** @import { ComponentClientTransformState, ComponentContext } from '../types' */ /** @import { Scope } from '../../../scope' */ -import { escape_html } from '../../../../../escaping.js'; import { cannot_be_set_statically, is_boolean_attribute, @@ -11,6 +10,7 @@ import { is_load_error_element, is_void } from '../../../../../utils.js'; +import { escape_html } from '../../../../../escaping.js'; import { dev, is_ignored, locator } from '../../../../state.js'; import { is_event_attribute, is_text_attribute } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; @@ -18,13 +18,12 @@ import { is_custom_element_node } from '../../../nodes.js'; import { clean_nodes, determine_namespace_for_children } from '../../utils.js'; import { build_getter, create_derived } from '../utils.js'; import { + get_attribute_name, build_attribute_value, build_class_directives, - build_set_attributes, build_style_directives, - get_attribute_name + build_set_attributes } from './shared/element.js'; -import { visit_event_attribute } from './shared/events.js'; import { process_children } from './shared/fragment.js'; import { build_render_statement, @@ -32,6 +31,7 @@ import { build_update, build_update_assignment } from './shared/utils.js'; +import { visit_event_attribute } from './shared/events.js'; /** * @param {AST.RegularElement} node @@ -352,32 +352,30 @@ export function RegularElement(node, context) { // special case — if an element that only contains text, we don't need // to descend into it if the text is non-reactive - const is_text = trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag'); - // in the rare case that we have static text that can't be inlined // (e.g. `{location}`), set `textContent` programmatically const use_text_content = - is_text && + trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag') && trimmed.every((node) => node.type === 'Text' || !node.metadata.expression.has_state) && - trimmed.some((node) => node.type === 'ExpressionTag' && !node.metadata.expression.can_inline); + trimmed.some((node) => node.type === 'ExpressionTag'); if (use_text_content) { - let { value } = build_template_chunk(trimmed, context.visit, child_state); - child_state.init.push( - b.stmt(b.assignment('=', b.member(context.state.node, 'textContent'), value)) + b.stmt( + b.assignment( + '=', + b.member(context.state.node, 'textContent'), + build_template_chunk(trimmed, context.visit, child_state).value + ) + ) ); } else { /** @type {Expression} */ let arg = context.state.node; // If `hydrate_node` is set inside the element, we need to reset it - // after the element has been hydrated (we don't need to reset if it's been inlined) - let needs_reset = !trimmed.every( - (node) => - node.type === 'Text' || - (node.type === 'ExpressionTag' && node.metadata.expression.can_inline) - ); + // after the element has been hydrated + let needs_reset = trimmed.some((node) => node.type !== 'Text'); // The same applies if it's a `