-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: add new remote function query.batch
#14272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 20 commits
1affeca
b70a394
bd0c519
41fad93
4819f97
a955c16
1b5f453
5da377d
e036a27
afdae97
6434cf3
e630de7
cbfa1cf
aa7cd56
910125c
2ca6eb1
dfa4cb4
44f60b8
d8d02bc
0986c29
4e81119
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@sveltejs/kit': minor | ||
--- | ||
|
||
feat: add new remote function `query.batch` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
export { command } from './command.svelte.js'; | ||
export { form } from './form.svelte.js'; | ||
export { prerender } from './prerender.svelte.js'; | ||
export { query } from './query.svelte.js'; | ||
export { query, query_batch } from './query.svelte.js'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,11 @@ | ||
/** @import { RemoteQueryFunction } from '@sveltejs/kit' */ | ||
/** @import { RemoteFunctionResponse } from 'types' */ | ||
import { app_dir, base } from '__sveltekit/paths'; | ||
import { remote_responses, started } from '../client.js'; | ||
import { app, goto, remote_responses, started } from '../client.js'; | ||
import { tick } from 'svelte'; | ||
import { create_remote_function, remote_request } from './shared.svelte.js'; | ||
import * as devalue from 'devalue'; | ||
import { HttpError, Redirect } from '@sveltejs/kit/internal'; | ||
|
||
/** | ||
* @param {string} id | ||
|
@@ -25,6 +28,97 @@ export function query(id) { | |
}); | ||
} | ||
|
||
/** | ||
* @param {string} id | ||
* @returns {(arg: any) => Query<any>} | ||
*/ | ||
export function query_batch(id) { | ||
/** @type {Map<string, Array<{resolve: (value: any) => void, reject: (error: any) => void}>>} */ | ||
let batching = new Map(); | ||
|
||
return create_remote_function(id, (cache_key, payload) => { | ||
return new Query(cache_key, () => { | ||
if (!started) { | ||
const result = remote_responses[cache_key]; | ||
if (result) { | ||
return result; | ||
} | ||
} | ||
|
||
// Collect all the calls to the same query in the same macrotask, | ||
// then execute them as one backend request. | ||
return new Promise((resolve, reject) => { | ||
// create_remote_function caches identical calls, but in case a refresh to the same query is called multiple times this function | ||
// is invoked multiple times with the same payload, so we need to deduplicate here | ||
const entry = batching.get(payload) ?? []; | ||
entry.push({ resolve, reject }); | ||
batching.set(payload, entry); | ||
|
||
if (batching.size > 1) return; | ||
|
||
// Wait for the next macrotask - don't use microtask as Svelte runtime uses these to collect changes and flush them, | ||
// and flushes could reveal more queries that should be batched. | ||
setTimeout(async () => { | ||
const batched = batching; | ||
batching = new Map(); | ||
|
||
try { | ||
const response = await fetch(`${base}/${app_dir}/remote/${id}`, { | ||
method: 'POST', | ||
body: JSON.stringify({ | ||
payloads: Array.from(batched.keys()) | ||
}), | ||
headers: { | ||
'Content-Type': 'application/json' | ||
} | ||
}); | ||
|
||
if (!response.ok) { | ||
throw new Error('Failed to execute batch query'); | ||
} | ||
|
||
const result = /** @type {RemoteFunctionResponse} */ (await response.json()); | ||
if (result.type === 'error') { | ||
throw new HttpError(result.status ?? 500, result.error); | ||
} | ||
|
||
if (result.type === 'redirect') { | ||
// TODO double-check this | ||
await goto(result.location); | ||
await new Promise((r) => setTimeout(r, 100)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's this about? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copied from the existing query code. Last time I specifically tested this I had to give the async function a bit of time before I rejected it to not cause problems with async Svelte. Not sure if that's still the case hence the TODO There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just tried deleting it, no tests failed. Not sure if that means it's safe to discard but my inclination is to do so, at most we should be using |
||
throw new Redirect(307, result.location); | ||
} | ||
|
||
const results = devalue.parse(result.result, app.decoders); | ||
|
||
// Resolve individual queries | ||
// Maps guarantee insertion order so we can do it like this | ||
let i = 0; | ||
|
||
for (const resolvers of batched.values()) { | ||
for (const { resolve, reject } of resolvers) { | ||
if (results[i].type === 'error') { | ||
reject(new HttpError(results[i].status, results[i].error)); | ||
} else { | ||
resolve(results[i].data); | ||
} | ||
} | ||
i++; | ||
} | ||
} catch (error) { | ||
// Reject all queries in the batch | ||
for (const resolver of batched.values()) { | ||
for (const { reject } of resolver) { | ||
reject(error); | ||
} | ||
} | ||
} | ||
}, 0); | ||
}); | ||
}); | ||
}); | ||
} | ||
|
||
/** | ||
* @template T | ||
* @implements {Partial<Promise<T>>} | ||
|
Uh oh!
There was an error while loading. Please reload this page.