Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/calm-mice-perform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: remove template expression inlining
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 });
}
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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' &&
Expand Down Expand Up @@ -84,12 +87,6 @@ export function Identifier(node, context) {
}
}

// no binding means global, and we can't inline e.g. `<span>{location}</span>`
// 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);
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
10 changes: 5 additions & 5 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
Expand All @@ -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;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,37 +1,37 @@
/** @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,
is_dom_property,
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';
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,
build_template_chunk,
build_update,
build_update_assignment
} from './shared/utils.js';
import { visit_event_attribute } from './shared/events.js';

/**
* @param {AST.RegularElement} node
Expand Down Expand Up @@ -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. `<span>{location}</span>`), 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 `<template>` element, since we need to
// set the value of `hydrate_node` to `node.content`
Expand All @@ -387,7 +385,7 @@ export function RegularElement(node, context) {
arg = b.member(arg, 'content');
}

process_children(trimmed, (is_text) => b.call('$.child', arg, is_text && b.true), node, {
process_children(trimmed, (is_text) => b.call('$.child', arg, is_text && b.true), true, {
...context,
state: child_state
});
Expand Down Expand Up @@ -587,44 +585,10 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
state.update.push(update);
}
return true;
}

// we need to special case textarea value because it's not an actual attribute
const can_inline =
(attribute.name !== 'value' || element.name !== 'textarea') &&
attribute.metadata.expression.can_inline;

if (can_inline) {
/** @type {Literal | undefined} */
let literal = undefined;

if (value.type === 'Literal') {
literal = value;
} else if (value.type === 'Identifier') {
const binding = context.state.scope.get(value.name);
if (binding && binding.initial?.type === 'Literal' && !binding.reassigned) {
literal = binding.initial;
}
}

if (literal && escape_html(literal.value, true) === String(literal.value)) {
if (is_boolean_attribute(name)) {
if (literal.value) {
context.state.template.push(` ${name}`);
}
} else {
context.state.template.push(` ${name}="`, value, '"');
}
} else {
context.state.template.push(
b.call('$.attr', b.literal(name), value, is_boolean_attribute(name) && b.true)
);
}
} else {
state.init.push(update);
return false;
}

return false;
}

/**
Expand Down
Loading
Loading