Add tests

Author: phi-bre

packages/kit/src/runtime/app/server/remote/query.js

@@ -72,7 +72,8 @@ export function query(validate_or_fn, maybe_fn) {
 
 		const { event, state } = get_request_store();
 
-		const get_remote_function_result = () => run_remote_function(event, state, false, arg, validate, fn);
+		const get_remote_function_result = () =>
+			run_remote_function(event, state, false, arg, validate, fn);
 
 		/** @type {Promise<any> & Partial<RemoteQuery<any>>} */
 		const promise = get_response(__, arg, state, get_remote_function_result);
@@ -90,11 +91,11 @@ export function query(validate_or_fn, maybe_fn) {
 				);
 			}
 
-			// TODO: Shouldn't this throw to signify that non-exported queries can't be set? Or still update the cache without touching refreshes?
 			if (__.id) {
-				const cache = get_cache(__, state);
 				const key = stringify_remote_arg(arg, state.transport);
-				refreshes[create_remote_cache_key(__.id, key)] = cache[key] = Promise.resolve(value);
+				const cache_key = create_remote_cache_key(__.id, key);
+				const cache = get_cache(__, state);
+				cache[key] = refreshes[cache_key] = Promise.resolve(value);
 			}
 		};
 
@@ -271,9 +272,10 @@ function batch(validate_or_fn, maybe_fn) {
 			if (__.id) {
 				const key = stringify_remote_arg(arg, state.transport);
 				const cache_key = create_remote_cache_key(__.id, key);
-				refreshes[cache_key] = Promise.resolve(value);
+				const cache = get_cache(__, state);
+				cache[key] = refreshes[cache_key] = Promise.resolve(value);
 			}
-		}
+		};
 
 		promise.refresh = () => {
 			const { state } = get_request_store();

packages/kit/test/apps/basics/src/routes/remote/+page.svelte

@@ -6,6 +6,7 @@
 		get_count,
 		set_count,
 		set_count_server_refresh,
+		set_count_server_refresh_after_read,
 		set_count_server_set,
 		resolve_deferreds
 	} from './query-command.remote.js';
@@ -67,6 +68,14 @@
 >
 	command (query server refresh)
 </button>
+<button
+	onclick={async () => {
+		command_result = await set_count_server_refresh_after_read(6);
+	}}
+	id="multiply-server-refresh-after-read-btn"
+>
+	command (query server refresh after read)
+</button>
 <button
 	onclick={async () => {
 		// slow, else test will not be able to see the override

packages/kit/test/apps/basics/src/routes/remote/batch/+page.svelte

@@ -1,15 +1,21 @@
 <script>
-	import { get_todo } from './batch.remote.js';
+	import {
+		get_todo,
+		set_todo_title_server_refresh,
+		reset_todos,
+		set_todo_title
+	} from './batch.remote.js';
 
 	const todoIds = ['1', '2', '1', 'error'];
+	const todos = todoIds.map((id) => ({ id, promise: get_todo(id) }));
 </script>
 
 <h1>Query Batch Test</h1>
 
 <ul>
-	{#each todoIds as id, idx}
+	{#each todos as { id, promise }, idx (idx)}
 		<li>
-			{#await get_todo(id)}
+			{#await promise}
 				<span id="batch-result-{idx + 1}">Loading todo {id}...</span>
 			{:then todo}
 				<span id="batch-result-{idx + 1}">{todo.title}</span>
@@ -21,3 +27,27 @@
 </ul>
 
 <button onclick={() => todoIds.forEach((id) => get_todo(id).refresh())}>refresh</button>
+<button
+	onclick={async () => {
+		await set_todo_title({ id: '1', title: 'Buy cat food' });
+	}}
+	id="batch-set-btn"
+>
+	set first todo
+</button>
+<button
+	onclick={async () => {
+		await set_todo_title_server_refresh({ id: '2', title: 'Walk the dog (refreshed)' });
+	}}
+	id="batch-refresh-btn"
+>
+	refresh second todo via command
+</button>
+<button
+	onclick={async () => {
+		await reset_todos();
+	}}
+	id="batch-reset-btn"
+>
+	reset todos
+</button>

packages/kit/test/apps/basics/src/routes/remote/batch/batch.remote.js

@@ -1,15 +1,43 @@
-import { query } from '$app/server';
+import { command, query } from '$app/server';
 import { error } from '@sveltejs/kit';
 
+/** @type {Array<[string, { id: string; title: string }]>} **/
+const INITIAL_TODOS = [
+	['1', { id: '1', title: 'Buy groceries' }],
+	['2', { id: '2', title: 'Walk the dog' }]
+];
+
+let todos = new Map(INITIAL_TODOS);
+
 export const get_todo = query.batch('unchecked', (ids) => {
-	if (JSON.stringify(ids) !== JSON.stringify(['1', '2', 'error'])) {
-		throw new Error(`Expected 3 IDs (deduplicated), got ${JSON.stringify(ids)}`);
+	if (new Set(ids).size !== ids.length) {
+		throw new Error(`batch queries must be deduplicated, but got ${JSON.stringify(ids)}`);
 	}
 
-	return (id) =>
-		id === '1'
-			? { id: '1', title: 'Buy groceries' }
-			: id === '2'
-				? { id: '2', title: 'Walk the dog' }
-				: error(404, { message: 'Not found' });
+	return (id) => {
+		if (id === 'error') return error(404, { message: 'Not found' });
+
+		return todos.get(id);
+	};
+});
+
+export const set_todo_title = command('unchecked', ({ id, title }) => {
+	const todo = { id, title };
+	todos.set(id, todo);
+	get_todo(id).set(todo);
+	return todo;
+});
+
+export const set_todo_title_server_refresh = command('unchecked', ({ id, title }) => {
+	const todo = { id, title };
+	todos.set(id, todo);
+	get_todo(id).refresh();
+	return todo;
+});
+
+export const reset_todos = command(() => {
+	todos = new Map(INITIAL_TODOS);
+	for (const [id, todo] of todos) {
+		get_todo(id).set({ ...todo });
+	}
 });

packages/kit/test/apps/basics/src/routes/remote/query-command.remote.js

@@ -36,6 +36,13 @@ export const set_count_server_refresh = command('unchecked', (c) => {
 	return c;
 });
 
+export const set_count_server_refresh_after_read = command('unchecked', async (c) => {
+	await get_count();
+	count = c;
+	await get_count().refresh();
+	return c;
+});
+
 export const set_count_server_set = command('unchecked', async (c) => {
 	get_count_called = false;
 	count = c;

packages/kit/test/apps/basics/test/client.test.js

@@ -1860,6 +1860,20 @@ test.describe('remote functions', () => {
 		expect(request_count).toBe(1); // no query refreshes, since that happens as part of the command response
 	});
 
+	test('command refresh after reading query reruns the query', async ({ page }) => {
+		await page.goto('/remote');
+		await expect(page.locator('#count-result')).toHaveText('0 / 0 (false)');
+
+		let request_count = 0;
+		page.on('request', (r) => (request_count += r.url().includes('/_app/remote') ? 1 : 0));
+
+		await page.click('#multiply-server-refresh-after-read-btn');
+		await expect(page.locator('#command-result')).toHaveText('6');
+		await expect(page.locator('#count-result')).toHaveText('6 / 6 (false)');
+		await page.waitForTimeout(100); // allow all requests to finish (in case there are query refreshes which shouldn't happen)
+		expect(request_count).toBe(1);
+	});
+
 	test('command does server-initiated single flight mutation (set)', async ({ page }) => {
 		await page.goto('/remote');
 		await expect(page.locator('#count-result')).toHaveText('0 / 0 (false)');
@@ -2027,6 +2041,36 @@ test.describe('remote functions', () => {
 		expect(request_count).toBe(1);
 	});
 
+	test('query.batch set updates cache without extra request', async ({ page }) => {
+		await page.goto('/remote/batch');
+		await page.click('#batch-reset-btn');
+		await expect(page.locator('#batch-result-1')).toHaveText('Buy groceries');
+
+		let request_count = 0;
+		const handler = (r) => (request_count += r.url().includes('/_app/remote') ? 1 : 0);
+		page.on('request', handler);
+
+		await page.click('#batch-set-btn');
+		await expect(page.locator('#batch-result-1')).toHaveText('Buy cat food');
+		await page.waitForTimeout(100); // allow all requests to finish
+		expect(request_count).toBe(1); // only the command request
+	});
+
+	test('query.batch refresh in command reuses single flight', async ({ page }) => {
+		await page.goto('/remote/batch');
+		await page.click('#batch-reset-btn');
+		await expect(page.locator('#batch-result-2')).toHaveText('Walk the dog');
+
+		let request_count = 0;
+		const handler = (r) => (request_count += r.url().includes('/_app/remote') ? 1 : 0);
+		page.on('request', handler);
+
+		await page.click('#batch-refresh-btn');
+		await expect(page.locator('#batch-result-2')).toHaveText('Walk the dog (refreshed)');
+		await page.waitForTimeout(100); // allow all requests to finish
+		expect(request_count).toBe(1); // only the command request
+	});
+
 	// TODO ditto
 	test('query works with transport', async ({ page }) => {
 		await page.goto('/remote/transport');