chore: deprecate top level promise await behaviour (#11175)

* chore: deprecate top level promise await behaviour * format * Update packages/kit/src/utils/promises.js Co-authored-by: Ben McCann <[email protected]> * more descriptive warning - we can take the developer directly to the file * while we're at it * ugh eslint * gah no good deed goes unpunished * ugh shut up eslint. no-one cares. go home --------- Co-authored-by: Simon H <[email protected]> Co-authored-by: Ben McCann <[email protected]> Co-authored-by: Rich Harris <[email protected]>
sveltejs · Dec 9, 2023 · b6a0be7 · b6a0be7
1 parent 15422d2
commit b6a0be7
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 11 deletions.
diff --git a/.changeset/long-cheetahs-explain.md b/.changeset/long-cheetahs-explain.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': minor
+---
+
+chore: deprecate top level promise await behaviour
diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js
@@ -559,7 +559,7 @@ export function create_client(app, target) {
 			} else {
 				data = (await node.universal.load.call(null, load_input)) ?? null;
 			}
-			data = data ? await unwrap_promises(data) : null;
+			data = data ? await unwrap_promises(data, route.id) : null;
 		}
 
 		return {

diff --git a/packages/kit/src/runtime/server/page/load_data.js b/packages/kit/src/runtime/server/page/load_data.js
@@ -125,9 +125,9 @@ export async function load_server_data({
 		url
 	});
 
-	const data = result ? await unwrap_promises(result) : null;
+	const data = result ? await unwrap_promises(result, node.server_id) : null;
 	if (__SVELTEKIT_DEV__) {
-		validate_load_response(data, /** @type {string} */ (event.route.id));
+		validate_load_response(data, node.server_id);
 	}
 
 	done = true;
@@ -181,9 +181,9 @@ export async function load_data({
 		parent
 	});
 
-	const data = result ? await unwrap_promises(result) : null;
+	const data = result ? await unwrap_promises(result, node.universal_id) : null;
 	if (__SVELTEKIT_DEV__) {
-		validate_load_response(data, /** @type {string} */ (event.route.id));
+		validate_load_response(data, node.universal_id);
 	}
 
 	return data;
@@ -360,12 +360,12 @@ async function stream_to_string(stream) {
 
 /**
  * @param {any} data
- * @param {string} [routeId]
+ * @param {string} [id]
  */
-function validate_load_response(data, routeId) {
+function validate_load_response(data, id) {
 	if (data != null && Object.getPrototypeOf(data) !== Object.prototype) {
 		throw new Error(
-			`a load function related to route '${routeId}' returned ${
+			`a load function in ${id} returned ${
 				typeof data !== 'object'
 					? `a ${typeof data}`
 					: data instanceof Response

diff --git a/packages/kit/src/utils/promises.js b/packages/kit/src/utils/promises.js
@@ -1,10 +1,54 @@
+import { DEV } from 'esm-env';
+
+/** @type {Set<string> | null} */
+let warned = null;
+
+// TODO v2: remove all references to unwrap_promises
+
 /**
  * Given an object, return a new object where all top level values are awaited
  *
  * @param {Record<string, any>} object
+ * @param {string | null} [id]
  * @returns {Promise<Record<string, any>>}
  */
-export async function unwrap_promises(object) {
+export async function unwrap_promises(object, id) {
+	if (DEV) {
+		/** @type {string[]} */
+		const promises = [];
+
+		for (const key in object) {
+			if (typeof object[key]?.then === 'function') {
+				promises.push(key);
+			}
+		}
+
+		if (promises.length > 0) {
+			if (!warned) warned = new Set();
+
+			const last = promises.pop();
+
+			const properties =
+				promises.length > 0
+					? `${promises.map((p) => `"${p}"`).join(', ')} and "${last}" properties`
+					: `"${last}" property`;
+
+			const location = id ? `the \`load\` function in ${id}` : 'a `load` function';
+
+			const description = promises.length > 0 ? 'are promises' : 'is a promise';
+
+			const message = `The top-level ${properties} returned from ${location} ${description}.`;
+
+			if (!warned.has(message)) {
+				console.warn(
+					`\n${message}\n\nIn SvelteKit 2.0, these will longer be awaited automatically. To get rid of this warning, await all promises included as top-level properties in \`load\` return values.\n`
+				);
+
+				warned.add(message);
+			}
+		}
+	}
+
 	for (const key in object) {
 		if (typeof object[key]?.then === 'function') {
 			return Object.fromEntries(

diff --git a/packages/kit/test/apps/basics/test/cross-platform/test.js b/packages/kit/test/apps/basics/test/cross-platform/test.js
@@ -236,8 +236,13 @@ test.describe('Errors', () => {
 			}
 
 			expect(await page.textContent('footer')).toBe('Custom layout');
+
+			const details = javaScriptEnabled
+				? "related to route '/errors/invalid-load-response'"
+				: 'in src/routes/errors/invalid-load-response/+page.js';
+
 			expect(await page.textContent('#message')).toBe(
-				'This is your custom error page saying: "a load function related to route \'/errors/invalid-load-response\' returned an array, but must return a plain object at the top level (i.e. `return {...}`)"'
+				`This is your custom error page saying: "a load function ${details} returned an array, but must return a plain object at the top level (i.e. \`return {...}\`)"`
 			);
 		});
 
@@ -254,8 +259,9 @@ test.describe('Errors', () => {
 			}
 
 			expect(await page.textContent('footer')).toBe('Custom layout');
+
 			expect(await page.textContent('#message')).toBe(
-				'This is your custom error page saying: "a load function related to route \'/errors/invalid-server-load-response\' returned an array, but must return a plain object at the top level (i.e. `return {...}`)"'
+				'This is your custom error page saying: "a load function in src/routes/errors/invalid-server-load-response/+page.server.js returned an array, but must return a plain object at the top level (i.e. `return {...}`)"'
 			);
 		});
 	}