chore: remove hoisted_promises (#16766)

* chore: remove hoisted_promises

* WIP optimise promises

* WIP

* fix <slot> with await in prop

* tweak

* fix type error

packages/svelte/src/compiler/phases/1-parse/utils/create.js

@@ -13,8 +13,7 @@ export function create_fragment(transparent = false) {
 			transparent,
 			dynamic: false,
 			has_await: false,
-			// name is added later, after we've done scope analysis
+			is_async: false
-			hoisted_promises: { id: b.id('$$promises'), promises: [] }
 		}
 	};
 }

packages/svelte/src/compiler/phases/2-analyze/index.js

@@ -542,8 +542,7 @@ export function analyze_component(root, source, options) {
 		source,
 		snippet_renderers: new Map(),
 		snippets: new Set(),
-		async_deriveds: new Set(),
+		async_deriveds: new Set()
-		hoisted_promises: new Map()
 	};
 
 	if (!runes) {

packages/svelte/src/compiler/phases/2-analyze/visitors/AwaitExpression.js

@@ -17,14 +17,6 @@ export function AwaitExpression(node, context) {
 			context.state.fragment.metadata.has_await = true;
 		}
 
-		if (context.state.fragment) {
-			// const len = context.state.fragment.metadata.hoisted_promises.promises.push(node.argument);
-			// context.state.analysis.hoisted_promises.set(
-			// 	node.argument,
-			// 	b.member(context.state.fragment.metadata.hoisted_promises.id, b.literal(len - 1), true)
-			// );
-		}
-
 		suspend = true;
 	}
 

packages/svelte/src/compiler/phases/2-analyze/visitors/Fragment.js

@@ -7,16 +7,4 @@
  */
 export function Fragment(node, context) {
 	context.next({ ...context.state, fragment: node });
-
-	// TODO this indicates whether the fragment contains an `await` expression (not inside
-	// a child fragment), which is necessary for ensuring that a `SnippetBlock` creates an
-	// async function in SSR. It feels like this is probably duplicative, but it's late
-	// and it works, so for now I'm doing it like this
-	node.metadata.is_async ||= node.metadata.hoisted_promises.promises.length > 0;
-
-	if (node.metadata.hoisted_promises.promises.length === 1) {
-		// if there's only one promise in this fragment, we don't need to de-waterfall it
-		context.state.analysis.hoisted_promises.delete(node.metadata.hoisted_promises.promises[0]);
-		node.metadata.hoisted_promises.promises.length = 0;
-	}
 }

packages/svelte/src/compiler/phases/3-transform/server/transform-server.js

@@ -10,7 +10,6 @@ import { dev, filename } from '../../../state.js';
 import { render_stylesheet } from '../css/index.js';
 import { AssignmentExpression } from './visitors/AssignmentExpression.js';
 import { AwaitBlock } from './visitors/AwaitBlock.js';
-import { AwaitExpression } from './visitors/AwaitExpression.js';
 import { CallExpression } from './visitors/CallExpression.js';
 import { ClassBody } from './visitors/ClassBody.js';
 import { Component } from './visitors/Component.js';
@@ -59,7 +58,6 @@ const global_visitors = {
 
 /** @type {ComponentVisitors} */
 const template_visitors = {
-	AwaitExpression,
 	AwaitBlock,
 	Component,
 	ConstTag,

packages/svelte/src/compiler/phases/3-transform/server/visitors/AwaitExpression.js

@@ -1,14 +0,0 @@
-/** @import { AwaitExpression } from 'estree' */
-/** @import { ComponentContext } from '../types.js' */
-
-/**
- * This is only registered for components, currently.
- * @param {AwaitExpression} node
- * @param {ComponentContext} context
- */
-export function AwaitExpression(node, context) {
-	const hoisted = context.state.analysis.hoisted_promises.get(node.argument);
-	if (hoisted) {
-		node.argument = hoisted;
-	}
-}

packages/svelte/src/compiler/phases/3-transform/server/visitors/Fragment.js

@@ -52,12 +52,6 @@ export function Fragment(node, context) {
 		/** @type {Statement[]} */
 		const statements = [];
 
-		if (node.metadata.hoisted_promises.promises.length > 0) {
-			statements.push(
-				b.const(node.metadata.hoisted_promises.id, b.array(node.metadata.hoisted_promises.promises))
-			);
-		}
-
 		statements.push(...state.init);
 		statements.push(...build_template(state.template));
 

packages/svelte/src/compiler/phases/3-transform/server/visitors/SlotElement.js

@@ -2,7 +2,12 @@
 /** @import { AST } from '#compiler' */
 /** @import { ComponentContext } from '../types.js' */
 import * as b from '#compiler/builders';
-import { empty_comment, build_attribute_value } from './shared/utils.js';
+import {
+	empty_comment,
+	build_attribute_value,
+	PromiseOptimiser,
+	call_child_payload
+} from './shared/utils.js';
 
 /**
  * @param {AST.SlotElement} node
@@ -15,13 +20,22 @@ export function SlotElement(node, context) {
 	/** @type {Expression[]} */
 	const spreads = [];
 
+	const optimiser = new PromiseOptimiser();
+
 	let name = b.literal('default');
 
 	for (const attribute of node.attributes) {
 		if (attribute.type === 'SpreadAttribute') {
-			spreads.push(/** @type {Expression} */ (context.visit(attribute)));
+			let expression = /** @type {Expression} */ (context.visit(attribute));
+			spreads.push(optimiser.transform(expression, attribute.metadata.expression));
 		} else if (attribute.type === 'Attribute') {
-			const value = build_attribute_value(attribute.value, context, false, true);
+			const value = build_attribute_value(
+				attribute.value,
+				context,
+				false,
+				true,
+				optimiser.transform
+			);
 
 			if (attribute.name === 'name') {
 				name = /** @type {Literal} */ (value);
@@ -50,5 +64,10 @@ export function SlotElement(node, context) {
 		fallback
 	);
 
-	context.state.template.push(empty_comment, b.stmt(slot), empty_comment);
+	const statement =
+		optimiser.expressions.length > 0
+			? call_child_payload(b.block([optimiser.apply(), b.stmt(slot)]), true)
+			: b.stmt(slot);
+
+	context.state.template.push(empty_comment, statement, empty_comment);
 }

packages/svelte/src/compiler/phases/3-transform/server/visitors/TitleElement.js

@@ -8,15 +8,6 @@ import { process_children, build_template, call_child_payload } from './shared/u
  * @param {ComponentContext} context
  */
 export function TitleElement(node, context) {
-	if (node.fragment.metadata.hoisted_promises.promises.length > 0) {
-		context.state.init.push(
-			b.const(
-				node.fragment.metadata.hoisted_promises.id,
-				b.array(node.fragment.metadata.hoisted_promises.promises)
-			)
-		);
-	}
-
 	// title is guaranteed to contain only text/expression tag children
 	const template = [b.literal('<title>')];
 	process_children(node.fragment.nodes, { ...context, state: { ...context.state, template } });

packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js

@@ -1,10 +1,16 @@
 /** @import { BlockStatement, Expression, Pattern, Property, SequenceExpression, Statement } from 'estree' */
 /** @import { AST } from '#compiler' */
 /** @import { ComponentContext } from '../../types.js' */
-import { empty_comment, build_attribute_value, call_child_payload } from './utils.js';
+import {
+	empty_comment,
+	build_attribute_value,
+	call_child_payload,
+	PromiseOptimiser
+} from './utils.js';
 import * as b from '#compiler/builders';
 import { is_element_node } from '../../../../nodes.js';
 import { dev } from '../../../../../state.js';
+import { get_attribute_chunks } from '../../../../../utils/ast.js';
 
 /**
  * @param {AST.Component | AST.SvelteComponent | AST.SvelteSelf} node
@@ -72,16 +78,26 @@ export function build_inline_component(node, expression, context) {
 		}
 	}
 
+	const optimiser = new PromiseOptimiser();
+
 	for (const attribute of node.attributes) {
 		if (attribute.type === 'LetDirective') {
 			if (!slot_scope_applies_to_itself) {
 				lets.default.push(attribute);
 			}
 		} else if (attribute.type === 'SpreadAttribute') {
-			props_and_spreads.push(/** @type {Expression} */ (context.visit(attribute)));
+			let expression = /** @type {Expression} */ (context.visit(attribute));
+			props_and_spreads.push(optimiser.transform(expression, attribute.metadata.expression));
 		} else if (attribute.type === 'Attribute') {
+			const value = build_attribute_value(
+				attribute.value,
+				context,
+				false,
+				true,
+				optimiser.transform
+			);
+
 			if (attribute.name.startsWith('--')) {
-				const value = build_attribute_value(attribute.value, context, false, true);
 				custom_css_props.push(b.init(attribute.name, value));
 				continue;
 			}
@@ -90,7 +106,6 @@ export function build_inline_component(node, expression, context) {
 				has_children_prop = true;
 			}
 
-			const value = build_attribute_value(attribute.value, context, false, true);
 			push_prop(b.prop('init', b.key(attribute.name), value));
 		} else if (attribute.type === 'BindDirective' && attribute.name !== 'this') {
 			if (attribute.expression.type === 'SequenceExpression') {
@@ -298,8 +313,7 @@ export function build_inline_component(node, expression, context) {
 		node.type === 'SvelteComponent' || (node.type === 'Component' && node.metadata.dynamic);
 
 	if (custom_css_props.length > 0) {
-		context.state.template.push(
+		statement = b.stmt(
-			b.stmt(
 			b.call(
 				'$.css_props',
 				b.id('$$payload'),
@@ -308,17 +322,20 @@ export function build_inline_component(node, expression, context) {
 				b.thunk(b.block([statement])),
 				dynamic && b.true
 			)
-			)
 		);
-	} else {
+	}
-		if (dynamic) {
+
+	if (optimiser.expressions.length > 0) {
+		statement = call_child_payload(b.block([optimiser.apply(), statement]), true);
+	}
+
+	if (dynamic && custom_css_props.length === 0) {
 		context.state.template.push(empty_comment);
 	}
 
 	context.state.template.push(statement);
 
-		if (!context.state.skip_hydration_boundaries) {
+	if (!context.state.skip_hydration_boundaries && custom_css_props.length === 0) {
 		context.state.template.push(empty_comment);
 	}
 }
-}

packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js

@@ -1,5 +1,5 @@
 /** @import { AssignmentOperator, Expression, Identifier, Node, Statement, BlockStatement } from 'estree' */
-/** @import { AST } from '#compiler' */
+/** @import { AST, ExpressionMetadata } from '#compiler' */
 /** @import { ComponentContext, ServerTransformState } from '../../types.js' */
 
 import { escape_html } from '../../../../../../escaping.js';
@@ -11,6 +11,7 @@ import {
 import * as b from '#compiler/builders';
 import { sanitize_template_string } from '../../../../../utils/sanitize_template_string.js';
 import { regex_whitespaces_strict } from '../../../../patterns.js';
+import { has_await } from '../../../../../utils/ast.js';
 
 /** Opens an if/each block, so that we can remove nodes in the case of a mismatch */
 export const block_open = b.literal(BLOCK_OPEN);
@@ -183,13 +184,15 @@ export function build_template(template, out = b.id('$$payload'), operator = 'pu
  * @param {ComponentContext} context
  * @param {boolean} trim_whitespace
  * @param {boolean} is_component
+ * @param {(expression: Expression, metadata: ExpressionMetadata) => Expression} transform
  * @returns {Expression}
  */
 export function build_attribute_value(
 	value,
 	context,
 	trim_whitespace = false,
-	is_component = false
+	is_component = false,
+	transform = (expression) => expression
 ) {
 	if (value === true) {
 		return b.true;
@@ -206,7 +209,10 @@ export function build_attribute_value(
 			return b.literal(is_component ? data : escape_html(data, true));
 		}
 
-		return /** @type {Expression} */ (context.visit(chunk.expression));
+		return transform(
+			/** @type {Expression} */ (context.visit(chunk.expression)),
+			chunk.metadata.expression
+		);
 	}
 
 	let quasi = b.quasi('', false);
@@ -224,7 +230,13 @@ export function build_attribute_value(
 				: node.data;
 		} else {
 			expressions.push(
-				b.call('$.stringify', /** @type {Expression} */ (context.visit(node.expression)))
+				b.call(
+					'$.stringify',
+					transform(
+						/** @type {Expression} */ (context.visit(node.expression)),
+						node.metadata.expression
+					)
+				)
 			);
 
 			quasi = b.quasi('', i + 1 === value.length);
@@ -269,3 +281,41 @@ export function build_getter(node, state) {
 export function call_child_payload(body, async) {
 	return b.stmt(b.call('$$payload.child', b.arrow([b.id('$$payload')], body, async)));
 }
+
+export class PromiseOptimiser {
+	/** @type {Expression[]} */
+	expressions = [];
+
+	/**
+	 *
+	 * @param {Expression} expression
+	 * @param {ExpressionMetadata} metadata
+	 */
+	transform = (expression, metadata) => {
+		if (metadata.has_await) {
+			const length = this.expressions.push(expression);
+			return b.id(`$$${length - 1}`);
+		}
+
+		return expression;
+	};
+
+	apply() {
+		if (this.expressions.length === 1) {
+			return b.const('$$0', this.expressions[0]);
+		}
+
+		const promises = b.array(
+			this.expressions.map((expression) => {
+				return expression.type === 'AwaitExpression' && !has_await(expression.argument)
+					? expression.argument
+					: b.call(b.thunk(expression, true));
+			})
+		);
+
+		return b.const(
+			b.array_pattern(this.expressions.map((_, i) => b.id(`$$${i}`))),
+			b.await(b.call('Promise.all', promises))
+		);
+	}
+}

packages/svelte/src/compiler/phases/types.d.ts

@@ -107,7 +107,6 @@ export interface ComponentAnalysis extends Analysis {
 	 * Every snippet that is declared locally
 	 */
 	snippets: Set<AST.SnippetBlock>;
-	hoisted_promises: Map<Expression, MemberExpression>;
 }
 
 declare module 'estree' {

packages/svelte/src/compiler/types/template.d.ts

@@ -59,7 +59,6 @@ export namespace AST {
 			has_await: boolean;
 			/** TODO document */
 			is_async: boolean;
-			hoisted_promises: { id: Identifier; promises: Expression[] };
 		};
 	}
 

packages/svelte/src/compiler/utils/ast.js

@@ -609,3 +609,19 @@ export function build_assignment_value(operator, left, right) {
 			? b.logical(/** @type {ESTree.LogicalOperator} */ (operator.slice(0, -1)), left, right)
 			: b.binary(/** @type {ESTree.BinaryOperator} */ (operator.slice(0, -1)), left, right);
 }
+
+/**
+ * @param {ESTree.Expression} expression
+ */
+export function has_await(expression) {
+	let has_await = false;
+
+	walk(expression, null, {
+		AwaitExpression(_node, context) {
+			has_await = true;
+			context.stop();
+		}
+	});
+
+	return has_await;
+}

packages/svelte/src/compiler/utils/builders.js

@@ -2,6 +2,7 @@
 import { walk } from 'zimmerframe';
 import { regex_is_valid_identifier } from '../phases/patterns.js';
 import { sanitize_template_string } from './sanitize_template_string.js';
+import { has_await } from './ast.js';
 
 /**
  * @param {Array<ESTree.Expression | ESTree.SpreadElement | null>} elements
@@ -443,16 +444,7 @@ export function thunk(expression, async = false) {
 export function unthunk(expression) {
 	// optimize `async () => await x()`, but not `async () => await x(await y)`
 	if (expression.async && expression.body.type === 'AwaitExpression') {
-		let has_await = false;
+		if (!has_await(expression.body.argument)) {
-
-		walk(expression.body.argument, null, {
-			AwaitExpression(_node, context) {
-				has_await = true;
-				context.stop();
-			}
-		});
-
-		if (!has_await) {
 			return unthunk(arrow(expression.params, expression.body.argument));
 		}
 	}