better errors for sync-in-async

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

@@ -40,6 +40,7 @@ import { TitleElement } from './visitors/TitleElement.js';
 import { UpdateExpression } from './visitors/UpdateExpression.js';
 import { VariableDeclaration } from './visitors/VariableDeclaration.js';
 import { SvelteBoundary } from './visitors/SvelteBoundary.js';
+import { call_child_payload } from './visitors/shared/utils.js';
 
 /** @type {Visitors} */
 const global_visitors = {
@@ -238,18 +239,12 @@ export function server_component(analysis, options) {
 	}
 
 	const component_block = b.block([
-		b.stmt(
-			b.call(
-				'$$payload.child',
-				b.arrow(
-					[b.id('$$payload')],
-					b.block([
-						.../** @type {Statement[]} */ (instance.body),
-						.../** @type {Statement[]} */ (template.body)
-					]),
-					analysis.suspends_without_fallback
-				)
-			)
+		call_child_payload(
+			b.block([
+				.../** @type {Statement[]} */ (instance.body),
+				.../** @type {Statement[]} */ (template.body)
+			]),
+			analysis.suspends_without_fallback
 		)
 	]);
 

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

@@ -2,7 +2,12 @@
 /** @import { ComponentContext, ComponentServerTransformState } from '../types.js' */
 import { clean_nodes, infer_namespace } from '../../utils.js';
 import * as b from '#compiler/builders';
-import { empty_comment, process_children, build_template } from './shared/utils.js';
+import {
+	empty_comment,
+	process_children,
+	build_template,
+	call_child_payload
+} from './shared/utils.js';
 
 /**
  * @param {AST.Fragment} node
@@ -49,12 +54,7 @@ export function Fragment(node, context) {
 				b.array(node.metadata.hoisted_promises.promises)
 			),
 			...state.init,
-			b.stmt(
-				b.call(
-					'$$payload.child',
-					b.arrow([b.id('$$payload')], b.block(build_template(state.template)), true)
-				)
-			)
+			call_child_payload(b.block(build_template(state.template)), true)
 		]);
 	}
 

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

@@ -12,7 +12,7 @@ import {
 	process_children,
 	build_template,
 	build_attribute_value,
-	wrap_in_child_payload
+	call_child_payload
 } from './shared/utils.js';
 
 /**
@@ -203,7 +203,7 @@ export function RegularElement(node, context) {
 		// TODO is this cast safe?
 		const elements = state.template.splice(template_start, Infinity);
 		state.template.push(
-			wrap_in_child_payload(
+			call_child_payload(
 				b.block(build_template(elements)),
 				// TODO this will always produce correct results (because it will produce an async function if the surrounding component is async)
 				// but it will false-positive and create unnecessary async functions (eg. when the component is async but the select element is not)

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

@@ -28,8 +28,7 @@ export function SvelteBoundary(node, context) {
 			);
 		}
 	} else {
-		// No pending snippet - render main content (may be async or sync)
-		context.state.template.push(b.literal(BLOCK_OPEN)); // <!--[-->
+		context.state.template.push(b.literal(BLOCK_OPEN));
 		context.state.template.push(/** @type {BlockStatement} */ (context.visit(node.fragment)));
 	}
 

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

@@ -1,7 +1,7 @@
 /** @import { AST } from '#compiler' */
 /** @import { ComponentContext } from '../types.js' */
 import * as b from '#compiler/builders';
-import { process_children, build_template } from './shared/utils.js';
+import { process_children, build_template, call_child_payload } from './shared/utils.js';
 
 /**
  * @param {AST.TitleElement} node
@@ -14,29 +14,20 @@ export function TitleElement(node, context) {
 	template.push(b.literal('</title>'));
 
 	context.state.init.push(
-		b.stmt(
-			b.call(
-				'$$payload.child',
-				// this nonsense is necessary so that the write to the title is as tightly scoped to a specific location
-				// in the async tree as possible. This lets us use `get_path` to compare this assignment to other assignments
-				// so that we can overwrite earlier assignments with later ones.
-				b.arrow(
-					[b.id('$$payload')],
-					b.block([
-						b.const('path', b.call('$$payload.get_path')),
-						b.let('title'),
-						...build_template(template, b.id('title'), '='),
-						b.stmt(
-							b.assignment(
-								'=',
-								b.id('$$payload.global.head.title'),
-								b.object([b.init('path', b.id('path')), b.init('value', b.id('title'))])
-							)
-						)
-					]),
-					node.metadata.has_await
+		call_child_payload(
+			b.block([
+				b.const('path', b.call('$$payload.get_path')),
+				b.let('title'),
+				...build_template(template, b.id('title'), '='),
+				b.stmt(
+					b.assignment(
+						'=',
+						b.id('$$payload.global.head.title'),
+						b.object([b.init('path', b.id('path')), b.init('value', b.id('title'))])
+					)
 				)
-			)
+			]),
+			node.metadata.has_await
 		)
 	);
 }

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

@@ -1,7 +1,7 @@
 /** @import { BlockStatement, Expression, Pattern, Property, SequenceExpression, Statement } from 'estree' */
 /** @import { AST } from '#compiler' */
 /** @import { ComponentContext } from '../../types.js' */
-import { empty_comment, build_attribute_value } from './utils.js';
+import { empty_comment, build_attribute_value, call_child_payload } from './utils.js';
 import * as b from '#compiler/builders';
 import { is_element_node } from '../../../../nodes.js';
 import { dev } from '../../../../../state.js';
@@ -234,16 +234,7 @@ export function build_inline_component(node, expression, context) {
 			// not necessary -- eg. when the component is asynchronous but the child content is not.
 			// May or may not be worth optimizing.
 			b.block([
-				b.stmt(
-					b.call(
-						'$$payload.child',
-						b.arrow(
-							[b.id('$$payload')],
-							b.block(block.body),
-							context.state.analysis.suspends_without_fallback
-						)
-					)
-				)
+				call_child_payload(b.block(block.body), context.state.analysis.suspends_without_fallback)
 			])
 		);
 

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

@@ -263,6 +263,6 @@ export function build_getter(node, state) {
  * @param {boolean} async
  * @returns {Statement}
  */
-export function wrap_in_child_payload(body, async) {
+export function call_child_payload(body, async) {
 	return b.stmt(b.call('$$payload.child', b.arrow([b.id('$$payload')], body, async)));
 }

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

@@ -107,7 +107,7 @@ export interface ComponentAnalysis extends Analysis {
 	 * Every snippet that is declared locally
 	 */
 	snippets: Set<AST.SnippetBlock>;
-	/** Whether the component uses `await` in a context that would require an `await` on the server. */
+	/** Whether the component uses `await` in a context that causes suspense outside of any boundary with a pending snippet. */
 	suspends_without_fallback: boolean;
 	hoisted_promises: Map<Expression, MemberExpression>;
 }

packages/svelte/src/internal/client/render.js

@@ -4,7 +4,6 @@ import { DEV } from 'esm-env';
 import {
 	clear_text_content,
 	create_text,
-	first_child,
 	get_first_child,
 	get_next_sibling,
 	init_operations
@@ -12,7 +11,7 @@ import {
 import { HYDRATION_END, HYDRATION_ERROR, HYDRATION_START } from '../../constants.js';
 import { active_effect } from './runtime.js';
 import { push, pop, component_context } from './context.js';
-import { component_root, branch } from './reactivity/effects.js';
+import { component_root } from './reactivity/effects.js';
 import {
 	hydrate_next,
 	hydrate_node,
@@ -29,7 +28,7 @@ import {
 import { reset_head_anchor } from './dom/blocks/svelte-head.js';
 import * as w from './warnings.js';
 import * as e from './errors.js';
-import { append, assign_nodes, comment } from './dom/template.js';
+import { assign_nodes } from './dom/template.js';
 import { is_passive_event } from '../../utils.js';
 import { COMMENT_NODE } from './constants.js';
 import { boundary } from './dom/blocks/boundary.js';

packages/svelte/src/internal/server/payload.js

@@ -55,12 +55,12 @@ export class Payload {
 	local;
 
 	/**
-	 * @param {TreeState} [global]
+	 * @param {TreeState} global
 	 * @param {{ select_value: string | undefined }} [local]
 	 * @param {Payload | undefined} [parent]
 	 * @param {PayloadType} [type]
 	 */
-	constructor(global = new TreeState(), local = { select_value: undefined }, parent, type) {
+	constructor(global, local = { select_value: undefined }, parent, type) {
 		this.global = global;
 		this.local = { ...local };
 		this.parent = parent;
@@ -79,6 +79,12 @@ export class Payload {
 		this.#out.push(child);
 		const result = render(child);
 		if (result instanceof Promise) {
+			if (this.global.mode === 'sync') {
+				// TODO more-proper error
+				throw new Error('Encountered an asynchronous component while rendering synchronously');
+			}
+			// just to avoid unhandled promise rejections -- we'll end up throwing in `then` if something fails
+			result.catch(() => {});
 			child.promises.initial = result;
 		}
 	}
@@ -135,8 +141,11 @@ export class Payload {
 	collect() {
 		const content = Payload.#collect_content(this.#out, this.type);
 		if (content instanceof Promise) {
-			// TODO is there a good way to report where this is? Probably by using some sort of loc or stack trace in `child` creation.
-			throw new Error('Encountered an asynchronous component while rendering synchronously');
+			// TODO improve message
+			// guess you could also end up here if you called `collect` in an async context but... just don't bro
+			throw new Error(
+				'invariant: should never reach this, as child throws when it encounters async work in a synchronous context'
+			);
 		}
 
 		return content;
@@ -153,6 +162,11 @@ export class Payload {
 	 * @param {Payload} other
 	 */
 	subsume(other) {
+		if (this.global.mode !== other.global.mode) {
+			// TODO message - this should be impossible though
+			throw new Error('invariant: a payload cannot switch modes');
+		}
+
 		this.global.subsume(other.global);
 		this.local = other.local;
 		this.#out = other.#out.map((item) => {
@@ -287,6 +301,9 @@ export class TreeState {
 	/** @type {TreeHeadState} */
 	#head;
 
+	/** @type {'sync' | 'async'} */
+	#mode;
+
 	get css() {
 		return this.#css;
 	}
@@ -299,17 +316,23 @@ export class TreeState {
 		return this.#head;
 	}
 
+	get mode() {
+		return this.#mode;
+	}
+
 	/**
+	 * @param {'sync' | 'async'} mode
 	 * @param {string} [id_prefix]
 	 */
-	constructor(id_prefix = '') {
+	constructor(mode, id_prefix = '') {
 		this.#uid = props_id_generator(id_prefix);
 		this.#css = new Set();
 		this.#head = new TreeHeadState(this.#uid);
+		this.#mode = mode;
 	}
 
 	copy() {
-		const state = new TreeState();
+		const state = new TreeState(this.#mode);
 		state.#css = new Set(this.#css);
 		state.#head = this.#head.copy();
 		state.#uid = this.#uid;
@@ -323,6 +346,7 @@ export class TreeState {
 		this.#css = other.#css;
 		this.#uid = other.#uid;
 		this.#head.subsume(other.#head);
+		this.#mode = other.#mode;
 	}
 }
 

packages/svelte/src/internal/server/payload.test.ts

@@ -2,7 +2,7 @@ import { assert, expect, test } from 'vitest';
 import { Payload, TreeState, TreeHeadState } from './payload.js';
 
 test('collects synchronous body content by default', () => {
-	const payload = new Payload();
+	const payload = new Payload(new TreeState('sync'));
 	payload.push('a');
 	payload.child(($$payload) => {
 		$$payload.push('b');
@@ -15,7 +15,7 @@ test('collects synchronous body content by default', () => {
 });
 
 test('child type switches content area (head vs body)', () => {
-	const payload = new Payload();
+	const payload = new Payload(new TreeState('sync'));
 	payload.push('a');
 	payload.child(($$payload) => {
 		$$payload.push('<title>T</title>');
@@ -28,7 +28,7 @@ test('child type switches content area (head vs body)', () => {
 });
 
 test('child inherits parent type when not specified', () => {
-	const parent = new Payload(undefined, undefined, undefined, 'head');
+	const parent = new Payload(new TreeState('sync'), undefined, undefined, 'head');
 	parent.push('<meta name="x"/>');
 	parent.child(($$payload) => {
 		$$payload.push('<style>/* css */</style>');
@@ -39,7 +39,7 @@ test('child inherits parent type when not specified', () => {
 });
 
 test('get_path returns the path indexes to a payload', () => {
-	const root = new Payload();
+	const root = new Payload(new TreeState('sync'));
 	let child_a: InstanceType<typeof Payload> | undefined;
 	let child_b: InstanceType<typeof Payload> | undefined;
 	let child_b_0: InstanceType<typeof Payload> | undefined;
@@ -62,8 +62,19 @@ test('get_path returns the path indexes to a payload', () => {
 	assert.deepEqual(child_b_0!.get_path(), [1, 0]);
 });
 
-test('awaiting payload resolves async children; collect throws on async', async () => {
-	const payload = new Payload();
+test('creating an async child in a sync context throws', () => {
+	const payload = new Payload(new TreeState('sync'));
+	payload.push('a');
+	expect(() =>
+		payload.child(async ($$payload) => {
+			await Promise.resolve();
+			$$payload.push('x');
+		})
+	).toThrow('Encountered an asynchronous component while rendering synchronously');
+});
+
+test('awaiting payload resolves async children', async () => {
+	const payload = new Payload(new TreeState('async'));
 	payload.push('a');
 	payload.child(async ($$payload) => {
 		await Promise.resolve();
@@ -71,17 +82,13 @@ test('awaiting payload resolves async children; collect throws on async', async
 	});
 	payload.push('y');
 
-	expect(() => payload.collect()).toThrow(
-		'Encountered an asynchronous component while rendering synchronously'
-	);
-
 	const { body, head } = await payload;
 	assert.equal(head, '');
 	assert.equal(body, 'axy');
 });
 
 test('then() allows awaiting payload to get aggregated content', async () => {
-	const payload = new Payload();
+	const payload = new Payload(new TreeState('async'));
 	payload.push('1');
 	payload.child(async ($$payload) => {
 		await Promise.resolve();
@@ -94,7 +101,7 @@ test('then() allows awaiting payload to get aggregated content', async () => {
 });
 
 test('compact synchronously aggregates a range and can transform into head/body', () => {
-	const payload = new Payload();
+	const payload = new Payload(new TreeState('sync'));
 	payload.push('a');
 	payload.push('b');
 	payload.push('c');
@@ -112,7 +119,7 @@ test('compact synchronously aggregates a range and can transform into head/body'
 });
 
 test('compact schedules followup when compaction input is async', async () => {
-	const payload = new Payload();
+	const payload = new Payload(new TreeState('async'));
 	payload.push('a');
 	payload.child(async ($$payload) => {
 		await Promise.resolve();
@@ -131,7 +138,7 @@ test('compact schedules followup when compaction input is async', async () => {
 });
 
 test('copy creates a deep copy of the tree and shares promises reference', () => {
-	const payload = new Payload();
+	const payload = new Payload(new TreeState('sync'));
 	let child_ref: InstanceType<typeof Payload> | undefined;
 	payload.child(($$payload) => {
 		child_ref = $$payload;
@@ -154,7 +161,7 @@ test('copy creates a deep copy of the tree and shares promises reference', () =>
 });
 
 test('local state is shallow-copied to children', () => {
-	const root = new Payload();
+	const root = new Payload(new TreeState('sync'));
 	root.local.select_value = 'A';
 	let child: InstanceType<typeof Payload> | undefined;
 	root.child(($$payload) => {
@@ -167,11 +174,11 @@ test('local state is shallow-copied to children', () => {
 });
 
 test('subsume replaces tree content and state from other', () => {
-	const a = new Payload(undefined, undefined, undefined, 'head');
+	const a = new Payload(new TreeState('async'), undefined, undefined, 'head');
 	a.push('<meta />');
 	a.local.select_value = 'A';
 
-	const b = new Payload();
+	const b = new Payload(new TreeState('async'));
 	b.child(async ($$payload) => {
 		await Promise.resolve();
 		$$payload.push('body');
@@ -183,21 +190,35 @@ test('subsume replaces tree content and state from other', () => {
 
 	a.subsume(b);
 
-	// content now matches b and is async
-	expect(() => a.collect()).toThrow(
-		'Encountered an asynchronous component while rendering synchronously'
-	);
 	assert.equal(a.type, 'body');
 	assert.equal(a.local.select_value, 'B');
 	assert.strictEqual(a.promises, b.promises);
 
 	// global state transferred
-	assert.ok(a.global.css.has({ hash: 'h', code: 'c' }) || [...a.global.css][0]?.hash === 'h');
+	assert.ok([...a.global.css][0]?.hash === 'h');
 	assert.equal(a.global.head.title.value, 'Title');
 });
 
+test('subsume refuses to switch modes', () => {
+	const a = new Payload(new TreeState('sync'), undefined, undefined, 'head');
+	a.push('<meta />');
+	a.local.select_value = 'A';
+
+	const b = new Payload(new TreeState('async'));
+	b.child(async ($$payload) => {
+		await Promise.resolve();
+		$$payload.push('body');
+	});
+	b.global.css.add({ hash: 'h', code: 'c' });
+	b.global.head.title = { path: [1], value: 'Title' };
+	b.local.select_value = 'B';
+	b.promises.initial = Promise.resolve();
+
+	expect(() => a.subsume(b)).toThrow('invariant: a payload cannot switch modes');
+});
+
 test('TreeState uid generator uses prefix and is shared by copy()', () => {
-	const state = new TreeState('id-');
+	const state = new TreeState('sync', 'id-');
 	assert.equal(state.uid(), 'id-s1');
 	const state_copy = state.copy();
 	assert.equal(state_copy.uid(), 'id-s2');