diff --git a/.changeset/cold-lamps-accept.md b/.changeset/cold-lamps-accept.md new file mode 100644 index 000000000000..9de7866ec805 --- /dev/null +++ b/.changeset/cold-lamps-accept.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: sort `{@const ...}` tags topologically in legacy mode diff --git a/packages/svelte/messages/compile-errors/template.md b/packages/svelte/messages/compile-errors/template.md index b35d83cf8f2f..2bb4197a1dec 100644 --- a/packages/svelte/messages/compile-errors/template.md +++ b/packages/svelte/messages/compile-errors/template.md @@ -96,6 +96,10 @@ > This type of directive is not valid on components +## const_tag_cycle + +> Cyclical dependency detected: %cycle% + ## const_tag_invalid_expression > {@const ...} must consist of a single variable declaration diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index df75245a682d..f4ce20c7401c 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -752,6 +752,16 @@ export function component_invalid_directive(node) { e(node, "component_invalid_directive", "This type of directive is not valid on components"); } +/** + * Cyclical dependency detected: %cycle% + * @param {null | number | NodeLike} node + * @param {string} cycle + * @returns {never} + */ +export function const_tag_cycle(node, cycle) { + e(node, "const_tag_cycle", `Cyclical dependency detected: ${cycle}`); +} + /** * {@const ...} must consist of a single variable declaration * @param {null | number | NodeLike} 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 11cabf0ee58e..0cdfd1e5dfaf 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 @@ -48,55 +48,6 @@ export function client_component(source, analysis, options) { scopes: analysis.template.scopes, hoisted: [b.import_all('$', 'svelte/internal/client')], node: /** @type {any} */ (null), // populated by the root node - // these should be set by create_block - if they're called outside, it's a bug - get before_init() { - /** @type {any[]} */ - const a = []; - a.push = () => { - throw new Error('before_init.push should not be called outside create_block'); - }; - return a; - }, - get init() { - /** @type {any[]} */ - const a = []; - a.push = () => { - throw new Error('init.push should not be called outside create_block'); - }; - return a; - }, - get update() { - /** @type {any[]} */ - const a = []; - a.push = () => { - throw new Error('update.push should not be called outside create_block'); - }; - return a; - }, - get after_update() { - /** @type {any[]} */ - const a = []; - a.push = () => { - throw new Error('after_update.push should not be called outside create_block'); - }; - return a; - }, - get template() { - /** @type {any[]} */ - const a = []; - a.push = () => { - throw new Error('template.push should not be called outside create_block'); - }; - return a; - }, - get locations() { - /** @type {any[]} */ - const a = []; - a.push = () => { - throw new Error('locations.push should not be called outside create_block'); - }; - return a; - }, legacy_reactive_statements: new Map(), metadata: { context: { @@ -110,7 +61,15 @@ export function client_component(source, analysis, options) { preserve_whitespace: options.preserveWhitespace, public_state: new Map(), private_state: new Map(), - in_constructor: false + in_constructor: false, + + // 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), + after_update: /** @type {any} */ (null), + template: /** @type {any} */ (null), + locations: /** @type {any} */ (null) }; const module = /** @type {import('estree').Program} */ ( 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 7ec0dd6c4a75..aec2edb68d88 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 @@ -49,9 +49,9 @@ export interface ComponentClientTransformState extends ClientTransformState { namespace: Namespace; bound_contenteditable: boolean; /** - * Stuff that is set within the children of one `create_block` that is relevant - * to said `create_block`. Shouldn't be destructured or otherwise spread unless - * inside `create_block` to keep the object reference intact (it's also nested + * Stuff that is set within the children of one `Fragment` visitor that is relevant + * to said fragment. Shouldn't be destructured or otherwise spread unless inside the + * `Fragment` visitor to keep the object reference intact (it's also nested * within `metadata` for this reason). */ context: { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index d53d7201cf46..9ad4b3d7dd2b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -847,18 +847,30 @@ function serialize_inline_component(node, component_name, context) { /** @type {import('estree').Property[]} */ const serialized_slots = []; for (const slot_name of Object.keys(children)) { - const body = create_block( - node, - node.fragment, - `${node.name}_${slot_name}`, - children[slot_name], - context + const block = /** @type {import('estree').BlockStatement} */ ( + context.visit( + { + ...node.fragment, + // @ts-expect-error + nodes: children[slot_name] + }, + { + ...context.state, + scope: + context.state.scopes.get(slot_name === 'default' ? children[slot_name][0] : node) ?? + context.state.scope + } + ) ); - if (body.length === 0) continue; + + if (block.body.length === 0) continue; const slot_fn = b.arrow( [b.id('$$anchor'), b.id('$$slotProps')], - b.block([...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), ...body]) + b.block([ + ...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), + ...block.body + ]) ); if (slot_name === 'default' && !has_children_prop) { @@ -1016,189 +1028,6 @@ function serialize_locations(locations) { ); } -/** - * Creates a new block which looks roughly like this: - * ```js - * // hoisted: - * const block_name = $.template(`...`); - * - * // for the main block: - * const id = block_name(); - * // init stuff and possibly render effect - * $.append($$anchor, id); - * ``` - * Adds the hoisted parts to `context.state.hoisted` and returns the statements of the main block. - * @param {import('#compiler').SvelteNode} parent - * @param {import('#compiler').Fragment} fragment - * @param {string} name - * @param {import('#compiler').SvelteNode[]} nodes - * @param {import('../types.js').ComponentContext} context - * @returns {import('estree').Statement[]} - */ -function create_block(parent, fragment, name, nodes, context) { - const namespace = infer_namespace(context.state.metadata.namespace, parent, nodes); - - const { hoisted, trimmed } = clean_nodes( - parent, - nodes, - context.path, - namespace, - context.state.preserve_whitespace, - context.state.options.preserveComments - ); - - if (hoisted.length === 0 && trimmed.length === 0) { - return []; - } - - const is_single_element = trimmed.length === 1 && trimmed[0].type === 'RegularElement'; - const is_single_child_not_needing_template = - trimmed.length === 1 && - (trimmed[0].type === 'SvelteFragment' || trimmed[0].type === 'TitleElement'); - - const template_name = context.state.scope.root.unique(name); - - /** @type {import('estree').Statement[]} */ - const body = []; - - /** @type {import('estree').Statement | undefined} */ - let close = undefined; - - /** @type {import('../types').ComponentClientTransformState} */ - const state = { - ...context.state, - scope: context.state.scopes.get(fragment) ?? context.state.scope, - before_init: [], - init: [], - update: [], - after_update: [], - template: [], - locations: [], - metadata: { - context: { - template_needs_import_node: false, - template_contains_script_tag: false - }, - namespace, - bound_contenteditable: context.state.metadata.bound_contenteditable - } - }; - - for (const node of hoisted) { - context.visit(node, state); - } - - /** - * @param {import('estree').Identifier} template_name - * @param {import('estree').Expression[]} args - */ - const add_template = (template_name, args) => { - let call = b.call(get_template_function(namespace, state), ...args); - if (context.state.options.dev) { - call = b.call( - '$.add_locations', - call, - b.member(b.id(context.state.analysis.name), b.id('filename')), - serialize_locations(state.locations) - ); - } - - context.state.hoisted.push(b.var(template_name, call)); - }; - - if (is_single_element) { - const element = /** @type {import('#compiler').RegularElement} */ (trimmed[0]); - - const id = b.id(context.state.scope.generate(element.name)); - - context.visit(element, { - ...state, - node: id - }); - - /** @type {import('estree').Expression[]} */ - const args = [b.template([b.quasi(state.template.join(''), true)], [])]; - - if (state.metadata.context.template_needs_import_node) { - args.push(b.literal(TEMPLATE_USE_IMPORT_NODE)); - } - - add_template(template_name, args); - - body.push(b.var(id, b.call(template_name)), ...state.before_init, ...state.init); - 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 > 0) { - const id = b.id(context.state.scope.generate('fragment')); - - const use_space_template = - 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, false, { - ...context, - state - }); - - body.push(b.var(id, b.call('$.text', b.id('$$anchor'))), ...state.before_init, ...state.init); - close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); - } else { - /** @type {(is_text: boolean) => import('estree').Expression} */ - const expression = (is_text) => - is_text ? b.call('$.first_child', id, b.true) : b.call('$.first_child', id); - - process_children(trimmed, expression, false, { ...context, state }); - - const use_comment_template = state.template.length === 1 && state.template[0] === ''; - - if (use_comment_template) { - // special case — we can use `$.comment` instead of creating a unique template - body.push(b.var(id, b.call('$.comment'))); - } else { - let flags = TEMPLATE_FRAGMENT; - - if (state.metadata.context.template_needs_import_node) { - flags |= TEMPLATE_USE_IMPORT_NODE; - } - - add_template(template_name, [ - b.template([b.quasi(state.template.join(''), true)], []), - b.literal(flags) - ]); - - body.push(b.var(id, b.call(template_name))); - } - - body.push(...state.before_init, ...state.init); - - close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); - } - } else { - body.push(...state.before_init, ...state.init); - } - - if (state.update.length > 0) { - body.push(serialize_render_stmt(state)); - } - - body.push(...state.after_update); - - if (close !== undefined) { - // It's important that close is the last statement in the block, as any previous statements - // could contain element insertions into the template, which the close statement needs to - // know of when constructing the list of current inner elements. - body.push(close); - } - - return body; -} - /** * * @param {import('#compiler').Namespace} namespace @@ -1688,7 +1517,184 @@ function serialize_template_literal(values, visit, state) { /** @type {import('../types').ComponentVisitors} */ export const template_visitors = { Fragment(node, context) { - const body = create_block(context.path.at(-1) ?? node, node, 'root', node.nodes, context); + // Creates a new block which looks roughly like this: + // ```js + // // hoisted: + // const block_name = $.template(`...`); + // + // // for the main block: + // const id = block_name(); + // // init stuff and possibly render effect + // $.append($$anchor, id); + // ``` + // Adds the hoisted parts to `context.state.hoisted` and returns the statements of the main block. + + const parent = context.path.at(-1) ?? node; + + const namespace = infer_namespace(context.state.metadata.namespace, parent, node.nodes); + + const { hoisted, trimmed } = clean_nodes( + parent, + node.nodes, + context.path, + namespace, + context.state, + context.state.preserve_whitespace, + context.state.options.preserveComments + ); + + if (hoisted.length === 0 && trimmed.length === 0) { + return b.block([]); + } + + const is_single_element = trimmed.length === 1 && trimmed[0].type === 'RegularElement'; + const is_single_child_not_needing_template = + trimmed.length === 1 && + (trimmed[0].type === 'SvelteFragment' || trimmed[0].type === 'TitleElement'); + + const template_name = context.state.scope.root.unique('root'); // TODO infer name from parent + + /** @type {import('estree').Statement[]} */ + const body = []; + + /** @type {import('estree').Statement | undefined} */ + let close = undefined; + + /** @type {import('../types').ComponentClientTransformState} */ + const state = { + ...context.state, + before_init: [], + init: [], + update: [], + after_update: [], + template: [], + locations: [], + metadata: { + context: { + template_needs_import_node: false, + template_contains_script_tag: false + }, + namespace, + bound_contenteditable: context.state.metadata.bound_contenteditable + } + }; + + for (const node of hoisted) { + context.visit(node, state); + } + + /** + * @param {import('estree').Identifier} template_name + * @param {import('estree').Expression[]} args + */ + const add_template = (template_name, args) => { + let call = b.call(get_template_function(namespace, state), ...args); + if (context.state.options.dev) { + call = b.call( + '$.add_locations', + call, + b.member(b.id(context.state.analysis.name), b.id('filename')), + serialize_locations(state.locations) + ); + } + + context.state.hoisted.push(b.var(template_name, call)); + }; + + if (is_single_element) { + const element = /** @type {import('#compiler').RegularElement} */ (trimmed[0]); + + const id = b.id(context.state.scope.generate(element.name)); + + context.visit(element, { + ...state, + node: id + }); + + /** @type {import('estree').Expression[]} */ + const args = [b.template([b.quasi(state.template.join(''), true)], [])]; + + if (state.metadata.context.template_needs_import_node) { + args.push(b.literal(TEMPLATE_USE_IMPORT_NODE)); + } + + add_template(template_name, args); + + body.push(b.var(id, b.call(template_name)), ...state.before_init, ...state.init); + 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 > 0) { + const id = b.id(context.state.scope.generate('fragment')); + + const use_space_template = + 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, false, { + ...context, + state + }); + + body.push( + b.var(id, b.call('$.text', b.id('$$anchor'))), + ...state.before_init, + ...state.init + ); + close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); + } else { + /** @type {(is_text: boolean) => import('estree').Expression} */ + const expression = (is_text) => + is_text ? b.call('$.first_child', id, b.true) : b.call('$.first_child', id); + + process_children(trimmed, expression, false, { ...context, state }); + + const use_comment_template = state.template.length === 1 && state.template[0] === ''; + + if (use_comment_template) { + // special case — we can use `$.comment` instead of creating a unique template + body.push(b.var(id, b.call('$.comment'))); + } else { + let flags = TEMPLATE_FRAGMENT; + + if (state.metadata.context.template_needs_import_node) { + flags |= TEMPLATE_USE_IMPORT_NODE; + } + + add_template(template_name, [ + b.template([b.quasi(state.template.join(''), true)], []), + b.literal(flags) + ]); + + body.push(b.var(id, b.call(template_name))); + } + + body.push(...state.before_init, ...state.init); + + close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); + } + } else { + body.push(...state.before_init, ...state.init); + } + + if (state.update.length > 0) { + body.push(serialize_render_stmt(state)); + } + + body.push(...state.after_update); + + if (close !== undefined) { + // It's important that close is the last statement in the block, as any previous statements + // could contain element insertions into the template, which the close statement needs to + // know of when constructing the list of current inner elements. + body.push(close); + } + return b.block(body); }, Comment(node, context) { @@ -2116,6 +2122,7 @@ export const template_visitors = { node.fragment.nodes, context.path, child_metadata.namespace, + state, node.name === 'script' || state.preserve_whitespace, state.options.preserveComments ); @@ -2229,16 +2236,15 @@ export const template_visitors = { } inner.push(...inner_context.state.after_update); inner.push( - ...create_block(node, node.fragment, 'dynamic_element', node.fragment.nodes, { - ...context, - state: { + .../** @type {import('estree').BlockStatement} */ ( + context.visit(node.fragment, { ...context.state, metadata: { ...context.state.metadata, namespace: determine_namespace_for_children(node, context.state.metadata.namespace) } - } - }) + }) + ).body ); const location = context.state.options.dev && locator(node.start); @@ -2456,8 +2462,7 @@ export const template_visitors = { } } - // TODO should use context.visit? - const children = create_block(node, node.body, 'each_block', node.body.nodes, context); + const block = /** @type {import('estree').BlockStatement} */ (context.visit(node.body)); const key_function = node.key ? b.arrow( @@ -2490,7 +2495,7 @@ export const template_visitors = { b.literal(each_type), each_node_meta.array_name ? each_node_meta.array_name : b.thunk(collection), key_function, - b.arrow([b.id('$$anchor'), item, index], b.block(declarations.concat(children))) + b.arrow([b.id('$$anchor'), item, index], b.block(declarations.concat(block.body))) ]; if (node.fallback) { @@ -3032,13 +3037,7 @@ export const template_visitors = { context.state.init.push(...lets); context.state.init.push( - ...create_block( - node, - node.fragment, - 'slot_template', - /** @type {import('#compiler').SvelteNode[]} */ (node.fragment.nodes), - context - ) + .../** @type {import('estree').BlockStatement} */ (context.visit(node.fragment)).body ); }, SlotElement(node, context) { @@ -3088,12 +3087,13 @@ export const template_visitors = { spreads.length === 0 ? b.object(props) : b.call('$.spread_props', b.object(props), ...spreads); + const fallback = node.fragment.nodes.length === 0 ? b.literal(null) : b.arrow( [b.id('$$anchor')], - b.block(create_block(node, node.fragment, 'fallback', node.fragment.nodes, context)) + /** @type {import('estree').BlockStatement} */ (context.visit(node.fragment)) ); const expression = is_default @@ -3111,7 +3111,7 @@ export const template_visitors = { '$.head', b.arrow( [b.id('$$anchor')], - b.block(create_block(node, node.fragment, 'head', node.fragment.nodes, context)) + /** @type {import('estree').BlockStatement} */ (context.visit(node.fragment)) ) ) ) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 0ac79f665554..0e5046e5b6c5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -238,63 +238,6 @@ function process_children(nodes, parent, { visit, state }) { } } -/** - * @param {import('#compiler').SvelteNode} parent - * @param {import('#compiler').Fragment} fragment - * @param {import('#compiler').SvelteNode[]} nodes - * @param {import('./types').ComponentContext} context - * @param {import('./types').Anchor} [anchor] - * @returns {import('estree').Statement[]} - */ -function create_block(parent, fragment, nodes, context, anchor) { - const namespace = infer_namespace(context.state.metadata.namespace, parent, nodes); - - const { hoisted, trimmed } = clean_nodes( - parent, - nodes, - context.path, - namespace, - context.state.preserve_whitespace, - context.state.options.preserveComments - ); - - if (hoisted.length === 0 && trimmed.length === 0 && !anchor) { - return []; - } - - /** @type {import('./types').ComponentServerTransformState} */ - const state = { - ...context.state, - scope: context.state.scopes.get(fragment) ?? context.state.scope, - init: [], - template: [], - metadata: { - namespace - } - }; - - for (const node of hoisted) { - context.visit(node, state); - } - - process_children(anchor ? [anchor, ...trimmed, anchor] : trimmed, parent, { - ...context, - state - }); - - /** @type {import('estree').Statement[]} */ - const body = []; - - if (state.template.length > 0) { - body.push(...state.init); - body.push(...serialize_template(state.template)); - } else { - body.push(...state.init); - } - - return body; -} - /** * @param {import('estree').VariableDeclarator} declarator * @param {import('../../scope').Scope} scope @@ -1088,12 +1031,30 @@ function serialize_inline_component(node, component_name, context) { const serialized_slots = []; for (const slot_name of Object.keys(children)) { - const body = create_block(node, node.fragment, children[slot_name], context); - if (body.length === 0) continue; + const block = /** @type {import('estree').BlockStatement} */ ( + context.visit( + { + ...node.fragment, + // @ts-expect-error + nodes: children[slot_name] + }, + { + ...context.state, + scope: + context.state.scopes.get(slot_name === 'default' ? children[slot_name][0] : node) ?? + context.state.scope + } + ) + ); + + if (block.body.length === 0) continue; const slot_fn = b.arrow( [b.id('$$payload'), b.id('$$slotProps')], - b.block([...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), ...body]) + b.block([ + ...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), + ...block.body + ]) ); if (slot_name === 'default' && !has_children_prop) { @@ -1271,7 +1232,46 @@ const javascript_visitors_legacy = { /** @type {import('./types').ComponentVisitors} */ const template_visitors = { Fragment(node, context) { - const body = create_block(context.path.at(-1) ?? node, node, node.nodes, context); + const parent = context.path.at(-1) ?? node; + const namespace = infer_namespace(context.state.metadata.namespace, parent, node.nodes); + + const { hoisted, trimmed } = clean_nodes( + parent, + node.nodes, + context.path, + namespace, + context.state, + context.state.preserve_whitespace, + context.state.options.preserveComments + ); + + if (hoisted.length === 0 && trimmed.length === 0) { + return b.block([]); + } + + /** @type {import('./types').ComponentServerTransformState} */ + const state = { + ...context.state, + init: [], + template: [], + metadata: { + namespace + } + }; + + for (const node of hoisted) { + context.visit(node, state); + } + + process_children(trimmed, parent, { ...context, state }); + + /** @type {import('estree').Statement[]} */ + const body = [...state.init]; + + if (state.template.length > 0) { + body.push(...serialize_template(state.template)); + } + return b.block(body); }, HtmlTag(node, context) { @@ -1382,6 +1382,7 @@ const template_visitors = { node.fragment.nodes, inner_context.path, metadata.namespace, + context.state, state.preserve_whitespace, state.options.preserveComments ); @@ -1475,10 +1476,12 @@ const template_visitors = { context.state.template.push(block_open); - const main = create_block(node, node.fragment, node.fragment.nodes, { - ...context, - state: { ...context.state, metadata } - }); + const main = /** @type {import('estree').BlockStatement} */ ( + context.visit(node.fragment, { + ...context.state, + metadata + }) + ); serialize_element_attributes(node, inner_context); @@ -1504,7 +1507,7 @@ const template_visitors = { ...serialize_template(inner_context.state.template) ]) ), - b.thunk(b.block(main)) + b.thunk(main) ) ) ) @@ -1543,11 +1546,7 @@ const template_visitors = { each.push(b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(block_open.value)))); - each.push( - .../** @type {import('estree').Statement[]} */ ( - create_block(node, node.body, children, context) - ) - ); + each.push(.../** @type {import('estree').BlockStatement} */ (context.visit(node.body)).body); each.push(b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(block_close.value)))); @@ -1561,16 +1560,20 @@ const template_visitors = { const close = b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(BLOCK_CLOSE))); if (node.fallback) { - const fallback = create_block(node, node.fallback, node.fallback.nodes, context); + const fallback = /** @type {import('estree').BlockStatement} */ ( + context.visit(node.fallback) + ); - fallback.push(b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(BLOCK_CLOSE_ELSE)))); + fallback.body.push( + b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(BLOCK_CLOSE_ELSE))) + ); state.template.push( t_statement( b.if( b.binary('!==', b.member(array_id, b.id('length')), b.literal(0)), b.block([for_loop, close]), - b.block(fallback) + fallback ) ) ); @@ -1582,23 +1585,22 @@ const template_visitors = { const state = context.state; state.template.push(block_open); - const consequent = create_block(node, node.consequent, node.consequent.nodes, context); - const alternate = node.alternate - ? create_block(node, node.alternate, node.alternate.nodes, context) - : []; + const test = /** @type {import('estree').Expression} */ (context.visit(node.test)); - consequent.push(b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(BLOCK_CLOSE)))); - alternate.push(b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(BLOCK_CLOSE_ELSE)))); + const consequent = /** @type {import('estree').BlockStatement} */ ( + context.visit(node.consequent) + ); - state.template.push( - t_statement( - b.if( - /** @type {import('estree').Expression} */ (context.visit(node.test)), - b.block(consequent), - b.block(alternate) - ) - ) + const alternate = node.alternate + ? /** @type {import('estree').BlockStatement} */ (context.visit(node.alternate)) + : b.block([]); + + consequent.body.push(b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(BLOCK_CLOSE)))); + alternate.body.push( + b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(BLOCK_CLOSE_ELSE))) ); + + state.template.push(t_statement(b.if(test, consequent, alternate))); }, AwaitBlock(node, context) { const state = context.state; @@ -1641,8 +1643,8 @@ const template_visitors = { KeyBlock(node, context) { const state = context.state; state.template.push(block_open); - const body = create_block(node, node.fragment, node.fragment.nodes, context); - state.template.push(t_statement(b.block(body))); + const block = /** @type {import('estree').BlockStatement} */ (context.visit(node.fragment)); + state.template.push(t_statement(block)); state.template.push(block_close); }, SnippetBlock(node, context) { @@ -1731,9 +1733,9 @@ const template_visitors = { } } - const body = create_block(node, node.fragment, node.fragment.nodes, context); + const block = /** @type {import('estree').BlockStatement} */ (context.visit(node.fragment)); - context.state.template.push(t_statement(b.block(body))); + context.state.template.push(t_statement(block)); }, TitleElement(node, context) { const state = context.state; @@ -1801,7 +1803,7 @@ const template_visitors = { const fallback = node.fragment.nodes.length === 0 ? b.literal(null) - : b.thunk(b.block(create_block(node, node.fragment, node.fragment.nodes, context))); + : b.thunk(/** @type {import('estree').BlockStatement} */ (context.visit(node.fragment))); const slot = b.call('$.slot', b.id('$$payload'), expression, props_expression, fallback); state.template.push(t_statement(b.stmt(slot))); @@ -1809,11 +1811,10 @@ const template_visitors = { }, SvelteHead(node, context) { const state = context.state; - const body = create_block(node, node.fragment, node.fragment.nodes, context); + const block = /** @type {import('estree').BlockStatement} */ (context.visit(node.fragment)); + state.template.push( - t_statement( - b.stmt(b.call('$.head', b.id('$$payload'), b.arrow([b.id('$$payload')], b.block(body)))) - ) + t_statement(b.stmt(b.call('$.head', b.id('$$payload'), b.arrow([b.id('$$payload')], block)))) ); }, // @ts-ignore: need to extract this out somehow @@ -2170,23 +2171,9 @@ export function server_component(analysis, options) { scopes: analysis.template.scopes, hoisted: [b.import_all('$', 'svelte/internal/server')], legacy_reactive_statements: new Map(), - // these should be set by create_block - if they're called outside, it's a bug - get init() { - /** @type {any[]} */ - const a = []; - a.push = () => { - throw new Error('init.push should not be called outside create_block'); - }; - return a; - }, - get template() { - /** @type {any[]} */ - const a = []; - a.push = () => { - throw new Error('template.push should not be called outside create_block'); - }; - return a; - }, + // these are set inside the `Fragment` visitor, and cannot be used until then + init: /** @type {any} */ (null), + template: /** @type {any} */ (null), metadata: { namespace: options.namespace }, diff --git a/packages/svelte/src/compiler/phases/3-transform/utils.js b/packages/svelte/src/compiler/phases/3-transform/utils.js index 1e5ef8dce1a3..251510ada74a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/utils.js @@ -4,7 +4,12 @@ import { regex_starts_with_whitespaces } from '../patterns.js'; import * as b from '../../utils/builders.js'; +import * as e from '../../errors.js'; import { walk } from 'zimmerframe'; +import { extract_identifiers } from '../../utils/ast.js'; +import check_graph_for_cycles from '../2-analyze/utils/check_graph_for_cycles.js'; +import is_reference from 'is-reference'; +import { set_scope } from '../scope.js'; /** * @param {import('estree').Node} node @@ -21,6 +26,111 @@ export function is_hoistable_function(node) { return false; } +/** + * Match Svelte 4 behaviour by sorting ConstTag nodes in topological order + * @param {import("#compiler").SvelteNode[]} nodes + * @param {import('./types.js').TransformState} state + */ +function sort_const_tags(nodes, state) { + /** + * @typedef {{ + * node: import('#compiler').ConstTag; + * ids: import('#compiler').Binding[]; + * deps: Set; + * }} Tag + */ + + const const_tags = []; + const other = []; + + /** @type {Map} */ + const tags = new Map(); + + const { _ } = set_scope(state.scopes); + + for (const node of nodes) { + if (node.type === 'ConstTag') { + const declaration = node.declaration.declarations[0]; + + /** @type {Tag} */ + const tag = { + node, + ids: extract_identifiers(declaration.id).map((id) => { + return /** @type {import('#compiler').Binding} */ (state.scope.get(id.name)); + }), + /** @type {Set} */ + deps: new Set() + }; + + for (const id of tag.ids) { + tags.set(id, tag); + } + + walk(declaration.init, state, { + _, + Identifier(node, context) { + const parent = /** @type {import('estree').Expression} */ (context.path.at(-1)); + + if (is_reference(node, parent)) { + const binding = context.state.scope.get(node.name); + if (binding) tag.deps.add(binding); + } + } + }); + + const_tags.push(tag); + } else { + other.push(node); + } + } + + if (const_tags.length === 0) { + return nodes; + } + + /** @type {Array<[import('#compiler').Binding, import('#compiler').Binding]>} */ + const edges = []; + + for (const tag of const_tags) { + for (const id of tag.ids) { + for (const dep of tag.deps) { + if (tags.has(dep)) { + edges.push([id, dep]); + } + } + } + } + + const cycle = check_graph_for_cycles(edges); + if (cycle?.length) { + const tag = /** @type {Tag} */ (tags.get(cycle[0])); + e.const_tag_cycle(tag.node, cycle.map((binding) => binding.node.name).join(' → ')); + } + + /** @type {import('#compiler').ConstTag[]} */ + const sorted = []; + + /** @param {Tag} tag */ + function add(tag) { + if (sorted.includes(tag.node)) { + return; + } + + for (const dep of tag.deps) { + const dep_tag = tags.get(dep); + if (dep_tag) add(dep_tag); + } + + sorted.push(tag.node); + } + + for (const tag of const_tags) { + add(tag); + } + + return [...sorted, ...other]; +} + /** * Extract nodes that are hoisted and trim whitespace according to the following rules: * - trim leading and trailing whitespace, regardless of surroundings @@ -32,6 +142,7 @@ export function is_hoistable_function(node) { * @param {import('#compiler').SvelteNode[]} nodes * @param {import('#compiler').SvelteNode[]} path * @param {import('#compiler').Namespace} namespace + * @param {import('./types.js').TransformState} state * @param {boolean} preserve_whitespace * @param {boolean} preserve_comments */ @@ -40,9 +151,17 @@ export function clean_nodes( nodes, path, namespace = 'html', + state, + // TODO give these defaults (state.options.preserveWhitespace and state.options.preserveComments). + // first, we need to make `Component(Client|Server)TransformState` inherit from a new `ComponentTransformState` + // rather than from `ClientTransformState` and `ServerTransformState` preserve_whitespace, preserve_comments ) { + if (!state.analysis.runes) { + nodes = sort_const_tags(nodes, state); + } + /** @type {import('#compiler').SvelteNode[]} */ const hoisted = []; diff --git a/packages/svelte/tests/compiler-errors/samples/const-tag-cyclical/_config.js b/packages/svelte/tests/compiler-errors/samples/const-tag-cyclical/_config.js new file mode 100644 index 000000000000..c08a60a8ba3f --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/const-tag-cyclical/_config.js @@ -0,0 +1,9 @@ +import { test } from '../../test'; + +export default test({ + error: { + code: 'const_tag_cycle', + message: 'Cyclical dependency detected: a → b → a', + position: [12, 26] + } +}); diff --git a/packages/svelte/tests/compiler-errors/samples/const-tag-cyclical/main.svelte b/packages/svelte/tests/compiler-errors/samples/const-tag-cyclical/main.svelte new file mode 100644 index 000000000000..26300090b608 --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/const-tag-cyclical/main.svelte @@ -0,0 +1,5 @@ +{#if true} + {@const a = b} + {@const b = a} +

hello {a}

+{/if} diff --git a/packages/svelte/tests/runtime-legacy/samples/const-tag-used-before-initialised copy/_config.js b/packages/svelte/tests/runtime-legacy/samples/const-tag-used-before-initialised copy/_config.js deleted file mode 100644 index b086f7cdf89e..000000000000 --- a/packages/svelte/tests/runtime-legacy/samples/const-tag-used-before-initialised copy/_config.js +++ /dev/null @@ -1,9 +0,0 @@ -import { test } from '../../test'; - -export default test({ - compileOptions: { - dev: true - }, - - error: "Cannot access 'c' before initialization" -}); diff --git a/packages/svelte/tests/runtime-legacy/samples/const-tag-used-before-initialised copy/main.svelte b/packages/svelte/tests/runtime-legacy/samples/const-tag-used-before-initialised copy/main.svelte deleted file mode 100644 index 746374b44fc5..000000000000 --- a/packages/svelte/tests/runtime-legacy/samples/const-tag-used-before-initialised copy/main.svelte +++ /dev/null @@ -1,8 +0,0 @@ - - -{#each array as a} - {@const b = a + c} - {@const c = b + a} -{/each} diff --git a/packages/svelte/tests/runtime-legacy/samples/const-tag/_config.js b/packages/svelte/tests/runtime-legacy/samples/const-tag/_config.js new file mode 100644 index 000000000000..9d625f89b801 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/const-tag/_config.js @@ -0,0 +1,5 @@ +import { test } from '../../test'; + +export default test({ + html: '

Hello worldworld!

' +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/const-tag/main.svelte b/packages/svelte/tests/runtime-legacy/samples/const-tag/main.svelte new file mode 100644 index 000000000000..3eaf38575519 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/const-tag/main.svelte @@ -0,0 +1,7 @@ + +{#if true} + {@const foo = bar} + {@const yoo = foo} + {@const bar = 'world'} +

Hello {bar}{yoo}!

+{/if}