diff --git a/.changeset/cuddly-walls-pretend.md b/.changeset/cuddly-walls-pretend.md new file mode 100644 index 0000000000..f51147a30c --- /dev/null +++ b/.changeset/cuddly-walls-pretend.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: omit unnecessary nullish coallescing in template expressions diff --git a/.changeset/cyan-games-cheat.md b/.changeset/cyan-games-cheat.md new file mode 100644 index 0000000000..d90901783b --- /dev/null +++ b/.changeset/cyan-games-cheat.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: more efficient template effect grouping diff --git a/README.md b/README.md index cfb1328495..7d2f718da7 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,9 @@ -[![Cybernetically enhanced web apps: Svelte](https://sveltejs.github.io/assets/banner.png)](https://svelte.dev) + + + + Svelte - web development for the rest of us + + [![license](https://img.shields.io/npm/l/svelte.svg)](LICENSE.md) [![Chat](https://img.shields.io/discord/457912077277855764?label=chat&logo=discord)](https://svelte.dev/chat) diff --git a/assets/banner.png b/assets/banner.png new file mode 100644 index 0000000000..3428b278bf Binary files /dev/null and b/assets/banner.png differ diff --git a/assets/banner_dark.png b/assets/banner_dark.png new file mode 100644 index 0000000000..1adba40d8e Binary files /dev/null and b/assets/banner_dark.png differ diff --git a/packages/svelte/README.md b/packages/svelte/README.md index 23e35fd4a7..c37617da30 100644 --- a/packages/svelte/README.md +++ b/packages/svelte/README.md @@ -1,4 +1,9 @@ -[![Cybernetically enhanced web apps: Svelte](https://sveltejs.github.io/assets/banner.png)](https://svelte.dev) + + + + Svelte - web development for the rest of us + + [![npm version](https://img.shields.io/npm/v/svelte.svg)](https://www.npmjs.com/package/svelte) [![license](https://img.shields.io/npm/l/svelte.svg)](LICENSE.md) [![Chat](https://img.shields.io/discord/457912077277855764?label=chat&logo=discord)](https://svelte.dev/chat) 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 75c79aab6a..41144fc74c 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js @@ -61,10 +61,6 @@ export function Attribute(node, context) { ) { continue; } - - node.metadata.expression.has_state ||= chunk.metadata.expression.has_state; - node.metadata.expression.has_call ||= chunk.metadata.expression.has_call; - node.metadata.expression.is_async ||= chunk.metadata.expression.is_async; } if (is_event_attribute(node)) { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 616376b012..93540db6a7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -172,9 +172,9 @@ export function client_component(analysis, options) { module_level_snippets: [], // these are set inside the `Fragment` visitor, and cannot be used until then - before_init: /** @type {any} */ (null), init: /** @type {any} */ (null), update: /** @type {any} */ (null), + expressions: /** @type {any} */ (null), after_update: /** @type {any} */ (null), template: /** @type {any} */ (null), locations: /** @type {any} */ (null) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 06309ac34e..a33b07d2b9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -46,14 +46,14 @@ export interface ComponentClientTransformState extends ClientTransformState { readonly events: Set; readonly is_instance: boolean; - /** Stuff that happens before the render effect(s) */ - readonly before_init: Statement[]; /** Stuff that happens before the render effect(s) */ readonly init: Statement[]; /** Stuff that happens inside the render effect */ readonly update: Statement[]; /** Stuff that happens after the render effect (control blocks, dynamic elements, bindings, actions, etc) */ readonly after_update: Statement[]; + /** Expressions used inside the render effect */ + readonly expressions: Expression[]; /** The HTML template string */ readonly template: Array; readonly locations: SourceLocation[]; 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 0755126e2a..a4da29743e 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 @@ -61,9 +61,9 @@ export function Fragment(node, context) { /** @type {ComponentClientTransformState} */ const state = { ...context.state, - before_init: [], init: [], update: [], + expressions: [], after_update: [], template: [], locations: [], @@ -125,18 +125,13 @@ export function Fragment(node, context) { add_template(template_name, args); - body.push(b.var(id, b.call(template_name)), ...state.before_init, ...state.init); + body.push(b.var(id, b.call(template_name))); close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } else if (is_single_child_not_needing_template) { context.visit(trimmed[0], state); - body.push(...state.before_init, ...state.init); } else if (trimmed.length === 1 && trimmed[0].type === 'Text') { const id = b.id(context.state.scope.generate('text')); - body.push( - b.var(id, b.call('$.text', b.literal(trimmed[0].data))), - ...state.before_init, - ...state.init - ); + body.push(b.var(id, b.call('$.text', b.literal(trimmed[0].data)))); close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } else if (trimmed.length > 0) { const id = b.id(context.state.scope.generate('fragment')); @@ -154,7 +149,7 @@ export function Fragment(node, context) { state }); - body.push(b.var(id, b.call('$.text')), ...state.before_init, ...state.init); + body.push(b.var(id, b.call('$.text'))); close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } else { if (is_standalone) { @@ -183,15 +178,13 @@ export function Fragment(node, context) { close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } - - body.push(...state.before_init, ...state.init); } - } else { - body.push(...state.before_init, ...state.init); } + body.push(...state.init); + if (state.update.length > 0) { - body.push(build_render_statement(state.update)); + body.push(build_render_statement(state)); } body.push(...state.after_update); 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 9446065919..458c44d4e6 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,4 +1,4 @@ -/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, ObjectExpression, Statement } from 'estree' */ +/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { SourceLocation } from '#shared' */ /** @import { ComponentClientTransformState, ComponentContext } from '../types' */ @@ -16,7 +16,7 @@ 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 { build_getter } from '../utils.js'; import { get_attribute_name, build_attribute_value, @@ -28,8 +28,8 @@ import { process_children } from './shared/fragment.js'; import { build_render_statement, build_template_chunk, - build_update, - build_update_assignment + build_update_assignment, + get_expression_id } from './shared/utils.js'; import { visit_event_attribute } from './shared/events.js'; @@ -409,7 +409,7 @@ export function RegularElement(node, context) { b.block([ ...child_state.init, ...element_state.init, - child_state.update.length > 0 ? build_render_statement(child_state.update) : b.empty, + child_state.update.length > 0 ? build_render_statement(child_state) : b.empty, ...child_state.after_update, ...element_state.after_update ]) @@ -536,7 +536,10 @@ function build_element_attribute_update_assignment( const name = get_attribute_name(element, attribute); const is_svg = context.state.metadata.namespace === 'svg' || element.name === 'svg'; const is_mathml = context.state.metadata.namespace === 'mathml'; - let { has_call, value } = build_attribute_value(attribute.value, context); + + let { value, has_state } = build_attribute_value(attribute.value, context, (value) => + get_expression_id(state, value) + ); if (name === 'autofocus') { state.init.push(b.stmt(b.call('$.autofocus', node_id, value))); @@ -557,15 +560,6 @@ function build_element_attribute_update_assignment( value = b.call('$.clsx', value); } - if (attribute.metadata.expression.has_state && has_call) { - // ensure we're not creating a separate template effect for this so that - // potential class directives are added to the same effect and therefore always apply - const id = b.id(state.scope.generate('class_derived')); - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); - has_call = false; - } - update = b.stmt( b.call( is_svg ? '$.set_svg_class' : is_mathml ? '$.set_mathml_class' : '$.set_class', @@ -605,14 +599,6 @@ function build_element_attribute_update_assignment( } else if (is_dom_property(name)) { update = b.stmt(b.assignment('=', b.member(node_id, name), value)); } else { - if (name === 'style' && attribute.metadata.expression.has_state && has_call) { - // ensure we're not creating a separate template effect for this so that - // potential style directives are added to the same effect and therefore always apply - const id = b.id(state.scope.generate('style_derived')); - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); - has_call = false; - } const callee = name.startsWith('xlink') ? '$.set_xlink_attribute' : '$.set_attribute'; update = b.stmt( b.call( @@ -625,12 +611,8 @@ function build_element_attribute_update_assignment( ); } - if (attribute.metadata.expression.has_state) { - if (has_call) { - state.init.push(build_update(update)); - } else { - state.update.push(update); - } + if (has_state) { + state.update.push(update); return true; } else { state.init.push(update); @@ -648,7 +630,7 @@ function build_element_attribute_update_assignment( function build_custom_element_attribute_update_assignment(node_id, attribute, context) { const state = context.state; const name = attribute.name; // don't lowercase, as we set the element's property, which might be case sensitive - let { has_call, value } = build_attribute_value(attribute.value, context); + let { value, has_state } = build_attribute_value(attribute.value, context); // We assume that noone's going to redefine the semantics of the class attribute on custom elements, i.e. it's still used for CSS classes if (name === 'class' && attribute.metadata.needs_clsx) { @@ -660,12 +642,10 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co const update = b.stmt(b.call('$.set_custom_element_data', node_id, b.literal(name), value)); - if (attribute.metadata.expression.has_state) { - if (has_call) { - state.init.push(build_update(update)); - } else { - state.update.push(update); - } + if (has_state) { + // this is different from other updates — it doesn't get grouped, + // because set_custom_element_data may not be idempotent + state.init.push(b.stmt(b.call('$.template_effect', b.thunk(update.expression)))); return true; } else { if (attribute.metadata.expression.is_async) { @@ -688,7 +668,9 @@ 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 } = build_attribute_value(attribute.value, context); + const { value, has_state } = build_attribute_value(attribute.value, context, (value) => + get_expression_id(state, value) + ); const inner_assignment = b.assignment( '=', @@ -722,7 +704,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont state.init.push(b.stmt(b.call('$.init_select', node_id, b.thunk(value)))); } - if (attribute.metadata.expression.has_state) { + if (has_state) { const id = state.scope.generate(`${node_id.name}_value`); build_update_assignment( state, 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 86734a07ab..f1b08acbc6 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 @@ -3,6 +3,7 @@ /** @import { ComponentContext } from '../types' */ import * as b from '../../../../utils/builders.js'; import { build_attribute_value } from './shared/element.js'; +import { memoize_expression } from './shared/utils.js'; /** * @param {AST.SlotElement} node @@ -29,13 +30,15 @@ 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 } = build_attribute_value(attribute.value, context); + const { value, has_state } = build_attribute_value(attribute.value, context, (value) => + memoize_expression(context.state, value) + ); if (attribute.name === 'name') { name = /** @type {Literal} */ (value); is_default = false; } else if (attribute.name !== 'slot') { - if (attribute.metadata.expression.has_state) { + if (has_state) { props.push(b.get(attribute.name, [b.return(value)])); } else { props.push(b.init(attribute.name, value)); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js index 48402ccc75..a9ed358cbb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js @@ -23,7 +23,7 @@ export function SvelteBoundary(node, context) { const expression = /** @type {Expression} */ (context.visit(chunk.expression, context.state)); - if (attribute.metadata.expression.has_state) { + if (chunk.metadata.expression.has_state) { props.properties.push(b.get(attribute.name, [b.return(expression)])); } else { props.properties.push(b.init(attribute.name, expression)); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js index ba66fe29d6..e275283655 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js @@ -1,12 +1,8 @@ -/** @import { BlockStatement, Expression, ExpressionStatement, Identifier, ObjectExpression, Statement } from 'estree' */ +/** @import { BlockStatement, Expression, ExpressionStatement, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types' */ -import { dev, is_ignored, locator } from '../../../../state.js'; -import { - get_attribute_expression, - is_event_attribute, - is_text_attribute -} from '../../../../utils/ast.js'; +import { dev, locator } from '../../../../state.js'; +import { is_text_attribute } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; import { determine_namespace_for_children } from '../../utils.js'; import { @@ -15,7 +11,7 @@ import { build_set_attributes, build_style_directives } from './shared/element.js'; -import { build_render_statement, build_update } from './shared/utils.js'; +import { build_render_statement } from './shared/utils.js'; /** * @param {AST.SvelteElement} node @@ -49,9 +45,9 @@ export function SvelteElement(node, context) { state: { ...context.state, node: element_id, - before_init: [], init: [], update: [], + expressions: [], after_update: [] } }; @@ -123,7 +119,7 @@ export function SvelteElement(node, context) { /** @type {Statement[]} */ const inner = inner_context.state.init; if (inner_context.state.update.length > 0) { - inner.push(build_render_statement(inner_context.state.update)); + inner.push(build_render_statement(inner_context.state)); } inner.push(...inner_context.state.after_update); inner.push( 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 0ab47afcbf..db607f2f32 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 @@ -5,7 +5,7 @@ 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, validate_binding } from '../shared/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'; import { determine_slot } from '../../../../../utils/slot.js'; @@ -132,7 +132,13 @@ export function build_component(node, component_name, context, anchor = context. } else if (attribute.type === 'Attribute') { if (attribute.name.startsWith('--')) { custom_css_props.push( - b.init(attribute.name, build_attribute_value(attribute.value, context).value) + b.init( + attribute.name, + build_attribute_value(attribute.value, context, (value) => + // TODO put the derived in the local block + memoize_expression(context.state, value) + ).value + ) ); continue; } @@ -145,9 +151,11 @@ export function build_component(node, component_name, context, anchor = context. has_children_prop = true; } - const { value } = build_attribute_value(attribute.value, context); + const { value, has_state } = build_attribute_value(attribute.value, context, (value) => + memoize_expression(context.state, value) + ); - if (attribute.metadata.expression.has_state) { + if (has_state) { let arg = value; // When we have a non-simple computation, anything other than an Identifier or Member expression, 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 e49dbaedb0..8fb6b8bdde 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,12 +1,12 @@ /** @import { Expression, Identifier, ObjectExpression } from 'estree' */ -/** @import { AST, Namespace } from '#compiler' */ +/** @import { AST } from '#compiler' */ /** @import { ComponentClientTransformState, ComponentContext } from '../../types' */ import { normalize_attribute } from '../../../../../../utils.js'; import { is_ignored } from '../../../../../state.js'; -import { get_attribute_expression, is_event_attribute } from '../../../../../utils/ast.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_template_chunk, build_update } from './utils.js'; +import { build_template_chunk, get_expression_id } from './utils.js'; /** * @param {Array} attributes @@ -28,15 +28,16 @@ export function build_set_attributes( is_custom_element, state ) { - let has_state = false; - let is_async = false; + let is_dynamic = false; /** @type {ObjectExpression['properties']} */ const values = []; for (const attribute of attributes) { if (attribute.type === 'Attribute') { - const { value } = build_attribute_value(attribute.value, context); + const { value, has_state } = build_attribute_value(attribute.value, context, (value) => + get_expression_id(context.state, value) + ); if ( is_event_attribute(attribute) && @@ -50,10 +51,10 @@ export function build_set_attributes( values.push(b.init(attribute.name, value)); } - has_state ||= attribute.metadata.expression.has_state; + is_dynamic ||= has_state; } else { // objects could contain reactive getters -> play it safe and always assume spread attributes are reactive - has_state = true; + is_dynamic = true; let value = /** @type {Expression} */ (context.visit(attribute)); @@ -64,14 +65,12 @@ export function build_set_attributes( } values.push(b.spread(value)); } - - is_async ||= attribute.metadata.expression.is_async; } const call = b.call( '$.set_attributes', element_id, - has_state ? attributes_id : b.literal(null), + is_dynamic ? attributes_id : b.literal(null), b.object(values), context.state.analysis.css.hash !== '' && b.literal(context.state.analysis.css.hash), preserve_attribute_case, @@ -79,7 +78,7 @@ export function build_set_attributes( is_ignored(element, 'hydration_attribute_changed') && b.true ); - if (has_state) { + if (is_dynamic) { context.state.init.push(b.let(attributes_id)); const update = b.stmt(b.assignment('=', attributes_id, call)); context.state.update.push(update); @@ -107,21 +106,14 @@ export function build_style_directives( const state = context.state; for (const directive of style_directives) { - const { has_state, has_call, is_async } = directive.metadata.expression; + const { has_state } = directive.metadata.expression; let value = directive.value === true ? build_getter({ name: directive.name, type: 'Identifier' }, context.state) - : build_attribute_value(directive.value, context).value; - - if (is_async) { - throw new Error('TODO'); - } else if (has_call) { - const id = b.id(state.scope.generate('style_directive')); - - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); - } + : build_attribute_value(directive.value, context, (value) => + get_expression_id(context.state, value) + ).value; const update = b.stmt( b.call( @@ -133,9 +125,7 @@ export function build_style_directives( ) ); - if (!is_attributes_reactive && has_call) { - state.init.push(build_update(update)); - } else if (is_attributes_reactive || has_state || has_call) { + if (has_state || is_attributes_reactive) { state.update.push(update); } else { state.init.push(update); @@ -159,23 +149,16 @@ export function build_class_directives( ) { const state = context.state; for (const directive of class_directives) { - const { has_state, has_call, is_async } = directive.metadata.expression; + const { has_state, has_call } = directive.metadata.expression; let value = /** @type {Expression} */ (context.visit(directive.expression)); - if (is_async) { - throw new Error('TODO'); - } else if (has_call) { - const id = b.id(state.scope.generate('class_directive')); - - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); + if (has_call) { + value = get_expression_id(state, value); } const update = b.stmt(b.call('$.toggle_class', element_id, b.literal(directive.name), value)); - if (!is_attributes_reactive && has_call) { - state.init.push(build_update(update)); - } else if (is_attributes_reactive || has_state || has_call) { + if (is_attributes_reactive || has_state) { state.update.push(update); } else { state.init.push(update); @@ -186,28 +169,30 @@ export function build_class_directives( /** * @param {AST.Attribute['value']} value * @param {ComponentContext} context - * @returns {{ value: Expression, has_state: boolean, has_call: boolean }} + * @param {(value: Expression) => Expression} memoize + * @returns {{ value: Expression, has_state: boolean }} */ -export function build_attribute_value(value, context) { +export function build_attribute_value(value, context, memoize = (value) => value) { if (value === true) { - return { has_state: false, has_call: false, value: b.literal(true) }; + return { value: b.literal(true), has_state: false }; } if (!Array.isArray(value) || value.length === 1) { const chunk = Array.isArray(value) ? value[0] : value; if (chunk.type === 'Text') { - return { has_state: false, has_call: false, value: b.literal(chunk.data) }; + return { value: b.literal(chunk.data), has_state: false }; } + let expression = /** @type {Expression} */ (context.visit(chunk.expression)); + return { - has_state: chunk.metadata.expression.has_state, - has_call: chunk.metadata.expression.has_call, - value: /** @type {Expression} */ (context.visit(chunk.expression)) + value: chunk.metadata.expression.has_call ? memoize(expression) : expression, + has_state: chunk.metadata.expression.has_state }; } - return build_template_chunk(value, context.visit, context.state); + return build_template_chunk(value, context.visit, context.state, memoize); } /** diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js index 7674fd1eb2..0b4ac87342 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js @@ -4,7 +4,7 @@ import { cannot_be_set_statically } from '../../../../../../utils.js'; import { is_event_attribute, is_text_attribute } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; -import { build_template_chunk, build_update } from './utils.js'; +import { build_template_chunk } from './utils.js'; /** * Processes an array of template nodes, joining sibling text/expression nodes @@ -69,7 +69,7 @@ export function process_children(nodes, initial, is_element, { visit, state }) { state.template.push(' '); - const { has_state, has_call, value } = build_template_chunk(sequence, visit, state); + const { has_state, value } = build_template_chunk(sequence, visit, state); // if this is a standalone `{expression}`, make sure we handle the case where // no text node was created because the expression was empty during SSR @@ -78,9 +78,7 @@ export function process_children(nodes, initial, is_element, { visit, state }) { const update = b.stmt(b.call('$.set_text', id, value)); - if (has_call && !within_bound_contenteditable) { - state.init.push(build_update(update)); - } else if (has_state && !within_bound_contenteditable) { + if (has_state && !within_bound_contenteditable) { state.update.push(update); } else { state.init.push(b.stmt(b.assignment('=', b.member(id, 'nodeValue'), value))); 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 528119b3fb..c4f81274d9 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 @@ -6,37 +6,95 @@ import { object } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; import { sanitize_template_string } from '../../../../../utils/sanitize_template_string.js'; import { regex_is_valid_identifier } from '../../../../patterns.js'; -import { create_derived } from '../../utils.js'; import is_reference from 'is-reference'; import { locator } from '../../../../../state.js'; +import { create_derived } from '../../utils.js'; + +/** + * @param {ComponentClientTransformState} state + * @param {Expression} value + */ +export function memoize_expression(state, value) { + const id = b.id(state.scope.generate('expression')); + state.init.push(b.const(id, create_derived(state, b.thunk(value)))); + return b.call('$.get', id); +} + +/** + * + * @param {ComponentClientTransformState} state + * @param {Expression} value + */ +export function get_expression_id(state, value) { + for (let i = 0; i < state.expressions.length; i += 1) { + if (compare_expressions(state.expressions[i], value)) { + return b.id(`$${i}`); + } + } + + return b.id(`$${state.expressions.push(value) - 1}`); +} + +/** + * Returns true of two expressions have an identical AST shape + * @param {Expression} a + * @param {Expression} b + */ +function compare_expressions(a, b) { + if (a.type !== b.type) { + return false; + } + + for (const key in a) { + if (key === 'type' || key === 'metadata' || key === 'loc' || key === 'start' || key === 'end') { + continue; + } + + const va = /** @type {any} */ (a)[key]; + const vb = /** @type {any} */ (b)[key]; + + if ((typeof va === 'object') !== (typeof vb === 'object')) { + return false; + } + + if (typeof va !== 'object' || va === null || vb === null) { + if (va !== vb) return false; + } else if (Array.isArray(va)) { + if (va.length !== vb.length) { + return false; + } + + if (va.some((v, i) => !compare_expressions(v, vb[i]))) { + return false; + } + } else if (!compare_expressions(va, vb)) { + return false; + } + } + + return true; +} /** * @param {Array} values * @param {(node: AST.SvelteNode, state: any) => any} visit * @param {ComponentClientTransformState} state - * @returns {{ value: Expression, has_state: boolean, has_call: boolean }} + * @param {(value: Expression) => Expression} memoize + * @returns {{ value: Expression, has_state: boolean }} */ -export function build_template_chunk(values, visit, state) { +export function build_template_chunk( + values, + visit, + state, + memoize = (value) => get_expression_id(state, value) +) { /** @type {Expression[]} */ const expressions = []; let quasi = b.quasi(''); const quasis = [quasi]; - let has_call = false; let has_state = false; - let is_async = false; - let should_memoize = false; - - for (const node of values) { - if (node.type === 'ExpressionTag') { - const metadata = node.metadata.expression; - - should_memoize ||= (has_call || is_async) && (metadata.has_call || metadata.is_async); - has_call ||= metadata.has_call; - has_state ||= metadata.has_state; - } - } for (let i = 0; i < values.length; i++) { const node = values[i]; @@ -48,26 +106,35 @@ export function build_template_chunk(values, visit, state) { quasi.value.cooked += node.expression.value + ''; } } else { - const expression = /** @type {Expression} */ (visit(node.expression, state)); + let value = /** @type {Expression} */ (visit(node.expression, state)); - if (node.metadata.expression.is_async) { - const id = b.id(state.scope.generate('expression')); - state.metadata.async.push({ id, expression: b.logical('??', expression, b.literal('')) }); + has_state ||= node.metadata.expression.has_state; - expressions.push(b.call(id)); - } else if (node.metadata.expression.has_call && should_memoize) { - const id = b.id(state.scope.generate('expression')); - state.init.push( - b.const(id, create_derived(state, b.thunk(b.logical('??', expression, b.literal(''))))) - ); + if (node.metadata.expression.has_call) { + value = memoize(value); + } - expressions.push(b.call('$.get', id)); - } else if (values.length === 1) { + 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). - return { value: expression, has_state, has_call }; + return { value, has_state }; } else { - expressions.push(b.logical('??', expression, b.literal(''))); + let expression = value; + // only add nullish coallescence if it hasn't been added already + if (value.type === 'LogicalExpression' && value.operator === '??') { + const { right } = value; + // `undefined` isn't a Literal (due to pre-ES5 shenanigans), so the only nullish literal is `null` + // however, you _can_ make a variable called `undefined` in a Svelte component, so we can't just treat it the same way + if (right.type !== 'Literal') { + expression = b.logical('??', value, b.literal('')); + } else if (right.value === null) { + // if they do something weird like `stuff ?? null`, replace `null` with empty string + value.right = b.literal(''); + } + } else { + expression = b.logical('??', value, b.literal('')); + } + expressions.push(expression); } quasi = b.quasi('', i + 1 === values.length); @@ -81,26 +148,27 @@ export function build_template_chunk(values, visit, state) { const value = b.template(quasis, expressions); - return { value, has_state, has_call }; -} - -/** - * @param {Statement} statement - */ -export function build_update(statement) { - const body = - statement.type === 'ExpressionStatement' ? statement.expression : b.block([statement]); - - return b.stmt(b.call('$.template_effect', b.thunk(body))); + return { value, has_state }; } /** - * @param {Statement[]} update + * @param {ComponentClientTransformState} state */ -export function build_render_statement(update) { - return update.length === 1 - ? build_update(update[0]) - : b.stmt(b.call('$.template_effect', b.thunk(b.block(update)))); +export function build_render_statement(state) { + return b.stmt( + b.call( + '$.template_effect', + b.arrow( + state.expressions.map((_, i) => b.id(`$${i}`)), + state.update.length === 1 && state.update[0].type === 'ExpressionStatement' + ? state.update[0].expression + : b.block(state.update) + ), + state.expressions.length > 0 && + b.array(state.expressions.map((expression) => b.thunk(expression))), + state.expressions.length > 0 && !state.analysis.runes && b.id('$.derived_safe_equal') + ) + ); } /** diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js index 22306989c8..b1e13e5cff 100644 --- a/packages/svelte/src/compiler/phases/nodes.js +++ b/packages/svelte/src/compiler/phases/nodes.js @@ -44,7 +44,6 @@ export function create_attribute(name, start, end, value) { name, value, metadata: { - expression: create_expression_metadata(), delegated: null, needs_clsx: false } diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 8be9aed177..fb60966895 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -479,7 +479,6 @@ export namespace AST { value: true | ExpressionTag | Array; /** @internal */ metadata: { - expression: ExpressionMetadata; /** May be set if this is an event attribute */ delegated: null | DelegatedEvent; /** May be `true` if this is a `class` attribute that needs `clsx` */ diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index abcb558c7f..f84a28acc4 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -43,7 +43,8 @@ import * as e from '../errors.js'; import { DEV } from 'esm-env'; import { define_property } from '../../shared/utils.js'; import { get_next_sibling } from '../dom/operations.js'; -import { destroy_derived } from './deriveds.js'; +import { derived, derived_safe_equal, destroy_derived } from './deriveds.js'; +import { legacy_mode_flag } from '../../flags/index.js'; /** * @param {'$effect' | '$effect.pre' | '$inspect'} rune @@ -343,16 +344,21 @@ export function render_effect(fn) { } /** - * @param {() => void | (() => void)} fn + * @param {(...expressions: any) => void | (() => void)} fn + * @param {Array<() => any>} thunks * @returns {Effect} */ -export function template_effect(fn) { +export function template_effect(fn, thunks = [], d = derived) { + const deriveds = thunks.map(d); + const effect = () => fn(...deriveds.map(get)); + if (DEV) { - define_property(fn, 'name', { + define_property(effect, 'name', { value: '{expression}' }); } - return block(fn); + + return block(effect); } /** diff --git a/packages/svelte/tests/runtime-legacy/samples/lifecycle-render-order-for-children/_config.js b/packages/svelte/tests/runtime-legacy/samples/lifecycle-render-order-for-children/_config.js index 73bfd09ceb..99f9681c4b 100644 --- a/packages/svelte/tests/runtime-legacy/samples/lifecycle-render-order-for-children/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/lifecycle-render-order-for-children/_config.js @@ -16,13 +16,13 @@ export default test({ test({ assert, compileOptions, component }) { assert.deepEqual(order, [ 'parent: beforeUpdate 0', - 'parent: render 0', '1: beforeUpdate 0', '1: render 0', '2: beforeUpdate 0', '2: render 0', '3: beforeUpdate 0', '3: render 0', + 'parent: render 0', '1: onMount 0', '1: afterUpdate 0', '2: onMount 0', @@ -39,13 +39,13 @@ export default test({ assert.deepEqual(order, [ 'parent: beforeUpdate 1', - 'parent: render 1', '1: beforeUpdate 1', '1: render 1', '2: beforeUpdate 1', '2: render 1', '3: beforeUpdate 1', '3: render 1', + 'parent: render 1', '1: afterUpdate 1', '2: afterUpdate 1', '3: afterUpdate 1', diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-2/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-2/_config.js index 1066d9a2df..cb8e648645 100644 --- a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-2/_config.js @@ -10,7 +10,6 @@ export default test({ assert.deepEqual(logs, [ 'parent: $effect.pre 0', 'parent: $effect.pre (2) 0', - 'parent: render 0', '1: $effect.pre 0', '1: $effect.pre (2) 0', '1: render 0', @@ -20,6 +19,7 @@ export default test({ '3: $effect.pre 0', '3: $effect.pre (2) 0', '3: render 0', + 'parent: render 0', '1: $effect 0', '2: $effect 0', '3: $effect 0', @@ -33,7 +33,6 @@ export default test({ assert.deepEqual(logs, [ 'parent: $effect.pre 1', 'parent: $effect.pre (2) 1', - 'parent: render 1', '1: $effect.pre 1', '1: $effect.pre (2) 1', '1: render 1', @@ -43,6 +42,7 @@ export default test({ '3: $effect.pre 1', '3: $effect.pre (2) 1', '3: render 1', + 'parent: render 1', '1: $effect 1', '2: $effect 1', '3: $effect 1', diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-3/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-3/_config.js index 55847c35a2..6c063bcb3e 100644 --- a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-3/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-3/_config.js @@ -8,13 +8,13 @@ export default test({ async test({ assert, component, logs }) { assert.deepEqual(logs, [ - 'parent: render 0', '1: $effect.pre 0', '1: render 0', '2: $effect.pre 0', '2: render 0', '3: $effect.pre 0', '3: render 0', + 'parent: render 0', '1: $effect 0', '2: $effect 0', '3: $effect 0', @@ -26,13 +26,13 @@ export default test({ flushSync(() => (component.n += 1)); assert.deepEqual(logs, [ - 'parent: render 1', '1: $effect.pre 1', '1: render 1', '2: $effect.pre 1', '2: render 1', '3: $effect.pre 1', '3: render 1', + 'parent: render 1', '1: $effect 1', '2: $effect 1', '3: $effect 1', diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-4/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-4/_config.js index 0cd7e15a37..29b0b67a52 100644 --- a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-4/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-4/_config.js @@ -10,7 +10,6 @@ export default test({ assert.deepEqual(logs, [ 'parent: $effect.pre 0', 'parent: nested $effect.pre 0', - 'parent: render 0', '1: $effect.pre 0', '1: nested $effect.pre 0', '1: render 0', @@ -20,6 +19,7 @@ export default test({ '3: $effect.pre 0', '3: nested $effect.pre 0', '3: render 0', + 'parent: render 0', '1: $effect 0', '2: $effect 0', '3: $effect 0', @@ -33,7 +33,6 @@ export default test({ assert.deepEqual(logs, [ 'parent: $effect.pre 1', 'parent: nested $effect.pre 1', - 'parent: render 1', '1: $effect.pre 1', '1: nested $effect.pre 1', '1: render 1', @@ -43,6 +42,7 @@ export default test({ '3: $effect.pre 1', '3: nested $effect.pre 1', '3: render 1', + 'parent: render 1', '1: $effect 1', '2: $effect 1', '3: $effect 1', diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children/_config.js index 19b8fb3938..3138ec7231 100644 --- a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children/_config.js @@ -9,13 +9,13 @@ export default test({ async test({ assert, component, logs }) { assert.deepEqual(logs, [ 'parent: $effect.pre 0', - 'parent: render 0', '1: $effect.pre 0', '1: render 0', '2: $effect.pre 0', '2: render 0', '3: $effect.pre 0', '3: render 0', + 'parent: render 0', '1: $effect 0', '2: $effect 0', '3: $effect 0', @@ -28,13 +28,13 @@ export default test({ assert.deepEqual(logs, [ 'parent: $effect.pre 1', - 'parent: render 1', '1: $effect.pre 1', '1: render 1', '2: $effect.pre 1', '2: render 1', '3: $effect.pre 1', '3: render 1', + 'parent: render 1', '1: $effect 1', '2: $effect 1', '3: $effect 1', diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js index ce77a27e19..d97a58bf40 100644 --- a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js @@ -11,23 +11,24 @@ export default function Main($$anchor) { var div = $.first_child(fragment); var svg = $.sibling(div, 2); var custom_element = $.sibling(svg, 2); - var div_1 = $.sibling(custom_element, 2); - $.template_effect(() => $.set_attribute(div_1, 'foobar', y())); + $.template_effect(() => $.set_custom_element_data(custom_element, 'fooBar', x)); + var div_1 = $.sibling(custom_element, 2); var svg_1 = $.sibling(div_1, 2); - - $.template_effect(() => $.set_attribute(svg_1, 'viewBox', y())); - var custom_element_1 = $.sibling(svg_1, 2); $.template_effect(() => $.set_custom_element_data(custom_element_1, 'fooBar', y())); - $.template_effect(() => { - $.set_attribute(div, 'foobar', x); - $.set_attribute(svg, 'viewBox', x); - $.set_custom_element_data(custom_element, 'fooBar', x); - }); + $.template_effect( + ($0) => { + $.set_attribute(div, 'foobar', x); + $.set_attribute(svg, 'viewBox', x); + $.set_attribute(div_1, 'foobar', $0); + $.set_attribute(svg_1, 'viewBox', $0); + }, + [y] + ); $.append($$anchor, fragment); } \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_config.js b/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_config.js new file mode 100644 index 0000000000..f47bee71df --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_config.js @@ -0,0 +1,3 @@ +import { test } from '../../test'; + +export default test({}); diff --git a/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_expected/client/index.svelte.js new file mode 100644 index 0000000000..332c909ebe --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_expected/client/index.svelte.js @@ -0,0 +1,34 @@ +import 'svelte/internal/disclose-version'; +import * as $ from 'svelte/internal/client'; + +var on_click = (_, count) => $.update(count); +var root = $.template(`

`, 1); + +export default function Nullish_coallescence_omittance($$anchor) { + let name = 'world'; + let count = $.state(0); + var fragment = root(); + var h1 = $.first_child(fragment); + + h1.textContent = `Hello, ${name ?? ''}!`; + + var b = $.sibling(h1, 2); + + b.textContent = `${1 ?? 'stuff'}${2 ?? 'more stuff'}${3 ?? 'even more stuff'}`; + + var button = $.sibling(b, 2); + + button.__click = [on_click, count]; + + var text = $.child(button); + + $.reset(button); + + var h1_1 = $.sibling(button, 2); + + h1_1.textContent = `Hello, ${name ?? 'earth' ?? ''}`; + $.template_effect(() => $.set_text(text, `Count is ${$.get(count) ?? ''}`)); + $.append($$anchor, fragment); +} + +$.delegate(['click']); \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_expected/server/index.svelte.js new file mode 100644 index 0000000000..8181bfd98e --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_expected/server/index.svelte.js @@ -0,0 +1,8 @@ +import * as $ from 'svelte/internal/server'; + +export default function Nullish_coallescence_omittance($$payload) { + let name = 'world'; + let count = 0; + + $$payload.out += `

Hello, ${$.escape(name)}!

${$.escape(1 ?? 'stuff')}${$.escape(2 ?? 'more stuff')}${$.escape(3 ?? 'even more stuff')}

Hello, ${$.escape(name ?? 'earth' ?? null)}

`; +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/index.svelte b/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/index.svelte new file mode 100644 index 0000000000..a67c574fee --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/index.svelte @@ -0,0 +1,8 @@ + +

Hello, {null}{name}!

+{1 ?? 'stuff'}{2 ?? 'more stuff'}{3 ?? 'even more stuff'} + +

Hello, {name ?? 'earth' ?? null}

\ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js index 2e0bd651da..d520d1ef24 100644 --- a/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js @@ -16,11 +16,9 @@ export default function Text_nodes_deriveds($$anchor) { } var p = root(); - const expression = $.derived(() => text1() ?? ''); - const expression_1 = $.derived(() => text2() ?? ''); var text = $.child(p); - $.template_effect(() => $.set_text(text, `${$.get(expression)}${$.get(expression_1)}`)); $.reset(p); + $.template_effect(($0, $1) => $.set_text(text, `${$0 ?? ''}${$1 ?? ''}`), [text1, text2]); $.append($$anchor, p); } \ No newline at end of file