better errors for sync-in-async

Author: elliott-with-the-longest-name-on-github

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,15 +4,14 @@ import { DEV } from 'esm-env';
 import {
 	clear_text_content,
 	create_text,
-	first_child,
 	get_first_child,
 	get_next_sibling,
 	init_operations
 } from './dom/operations.js';
 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;
 	}
 }