Skip to content

Commit 372a6a6

Browse files
Rich-Harrisvercel[bot]elliott-with-the-longest-name-on-github
authored
fix: dedupe remote data (#15991)
I thought that sveltejs/svelte#18406 would be necessary to do this, but I was wrong — it actually seems easier and simpler to bypass `hydratable` altogether. Instead, we serialize all the remote data in one go, allowing devalue to do its thing and deduplicate everything. (I still think it's worth merging that PR.) That way, if you have (for example) a `getUser(): Promise<User | null>` and a `requireUser(): Promise<User>` that calls `getUser` internally (and redirects if it returns `null`), the `User` object is only serialized once. I can't remember exactly why we chose to use `hydratable` in the first place. Perhaps it was a lifecycle thing — we wanted to be careful about data being stale by the time it was read? In which case I think that has changed now that query lifecycle is determined by the garbage collector. Maybe @elliott-with-the-longest-name-on-github remembers. --- ### Please don't delete this checklist! Before submitting the PR, please make sure you do the following: - [ ] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] This message body should clearly illustrate what problems it solves. - [x] Ideally, include a test that fails without this PR but passes with it. ### Tests - [ ] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check` ### Changesets - [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running `pnpm changeset` and following the prompts. Changesets that add features should be `minor` and those that fix bugs should be `patch`. Please prefix changeset messages with `feat:`, `fix:`, or `chore:`. ### Edits - [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed. --------- Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: Elliott Johnson <hello@ell.iott.dev>
1 parent 607713e commit 372a6a6

64 files changed

Lines changed: 1698 additions & 946 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.changeset/sad-frogs-wink.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sveltejs/kit': patch
3+
---
4+
5+
fix: dedupe remote data

packages/kit/src/core/postbuild/prerender.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,8 @@ async function prerender({ hash, out, manifest_path, metadata, verbose, env }) {
204204

205205
const seen = new Set();
206206
const written = new Set();
207+
208+
/** @type {Map<string, Promise<any>>} */
207209
const remote_responses = new Map();
208210

209211
/** @type {Map<string, Set<string>>} */

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,6 @@ export function command(validate_or_fn, maybe_fn) {
7777
);
7878
}
7979

80-
state.remote.refreshes ??= new Map();
81-
state.remote.reconnects ??= new Map();
82-
8380
const promise = Promise.resolve(
8481
run_remote_function(event, state, true, () => validate(arg), fn)
8582
);

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

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
normalize_issue,
1212
flatten_issues
1313
} from '../../../form-utils.js';
14-
import { get_cache, run_remote_function } from './shared.js';
14+
import { get_cache, get_implicit_lookup, run_remote_function } from './shared.js';
1515
import { ValidationError } from '@sveltejs/kit/internal';
1616

1717
/**
@@ -136,9 +136,6 @@ export function form(validate_or_fn, maybe_fn) {
136136
data = validated.value;
137137
}
138138

139-
state.remote.refreshes ??= new Map();
140-
state.remote.reconnects ??= new Map();
141-
142139
const issue = create_issues();
143140

144141
try {
@@ -161,7 +158,12 @@ export function form(validate_or_fn, maybe_fn) {
161158
// We don't need to care about args or deduplicating calls, because uneval results are only relevant in full page reloads
162159
// where only one form submission is active at the same time
163160
if (!event.isRemoteRequest) {
164-
get_cache(__, state)[''] ??= { serialize: true, data: output };
161+
const cache = get_cache(__, state);
162+
cache[''] ??= output;
163+
164+
// register under the client-side action id so the output is serialized
165+
// into the page, allowing the hydrated client to restore `result`/`issues`/`input`
166+
get_implicit_lookup(__, state)[__.action_id ?? __.id] = () => cache[''];
165167
}
166168

167169
return output;
@@ -177,28 +179,30 @@ export function form(validate_or_fn, maybe_fn) {
177179

178180
Object.defineProperty(instance, 'fields', {
179181
get() {
182+
// the form instance is created once per module and shared across requests,
183+
// so the current request's state has to be resolved at access time
180184
return create_field_proxy(
181185
{},
182-
() => get_cache(__)?.['']?.data?.input ?? {},
186+
() => get_cache(__, get_request_store().state)?.['']?.input ?? {},
183187
(path, value) => {
184-
const cache = get_cache(__);
188+
const cache = get_cache(__, get_request_store().state);
185189
const entry = cache[''];
186190

187-
if (entry?.data?.submission) {
191+
if (entry?.submission) {
188192
// don't override a submission
189193
return;
190194
}
191195

192196
if (path.length === 0) {
193-
(cache[''] ??= { serialize: true, data: {} }).data.input = value;
197+
(cache[''] ??= {}).input = value;
194198
return;
195199
}
196200

197-
const input = entry?.data?.input ?? {};
201+
const input = entry?.input ?? {};
198202
deep_set(input, path.map(String), value);
199-
(cache[''] ??= { serialize: true, data: {} }).data.input = input;
203+
(cache[''] ??= {}).input = input;
200204
},
201-
() => flatten_issues(get_cache(__)?.['']?.data?.issues ?? [])
205+
() => flatten_issues(get_cache(__, get_request_store().state)?.['']?.issues ?? [])
202206
);
203207
}
204208
});
@@ -220,7 +224,7 @@ export function form(validate_or_fn, maybe_fn) {
220224
Object.defineProperty(instance, 'result', {
221225
get() {
222226
try {
223-
return get_cache(__)?.['']?.data?.result;
227+
return get_cache(__, get_request_store().state)?.['']?.result;
224228
} catch {
225229
return undefined;
226230
}
@@ -264,6 +268,7 @@ export function form(validate_or_fn, maybe_fn) {
264268
if (!instance) {
265269
instance = create_instance(key);
266270
instance.__.id = `${__.id}/${encodeURIComponent(JSON.stringify(key))}`;
271+
instance.__.action_id = `${__.id}/${JSON.stringify(key)}`;
267272
instance.__.name = __.name;
268273

269274
state.remote.forms.set(cache_key, instance);

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

Lines changed: 28 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
/** @import { RemoteResource, RemotePrerenderFunction } from '@sveltejs/kit' */
22
/** @import { RemotePrerenderInputsGenerator, RemotePrerenderInternals, MaybePromise } from 'types' */
33
/** @import { StandardSchemaV1 } from '@standard-schema/spec' */
4-
import { error, json } from '@sveltejs/kit';
4+
import { json, error } from '@sveltejs/kit';
55
import { DEV } from 'esm-env';
66
import { get_request_store } from '@sveltejs/kit/internal/server';
77
import { stringify, stringify_remote_arg } from '../../../shared.js';
88
import { noop } from '../../../../utils/functions.js';
99
import { app_dir, base } from '$app/paths/internal/server';
1010
import {
1111
create_validator,
12-
get_cache,
1312
get_response,
1413
parse_remote_response,
1514
run_remote_function
@@ -89,51 +88,46 @@ export function prerender(validate_or_fn, fn_or_options, maybe_options) {
8988

9089
/** @type {RemotePrerenderFunction<Input, Output> & { __: RemotePrerenderInternals }} */
9190
const wrapper = (arg) => {
91+
const { event, state } = get_request_store();
92+
const payload = stringify_remote_arg(arg, state.transport);
93+
94+
// `get_response` (as opposed to bare `get_cache`) also registers the call in the
95+
// implicit lookup, so that the result is inlined into the page payload (`data.p`)
96+
// and the client doesn't need to fetch it again upon hydration
9297
/** @type {Promise<Output> & Partial<RemoteResource<Output>>} */
93-
const promise = (async () => {
94-
const { event, state } = get_request_store();
95-
const payload = stringify_remote_arg(arg, state.transport);
98+
const promise = get_response(__, payload, state, async () => {
9699
const id = __.id;
97100
const url = `${base}/${app_dir}/remote/${id}${payload ? `/${payload}` : ''}`;
98101

99102
if (!state.prerendering && !DEV && !event.isRemoteRequest) {
100103
try {
101-
return await get_response(__, payload, state, async () => {
102-
const cache = get_cache(__, state);
103-
104-
// TODO adapters can provide prerendered data more efficiently than
105-
// fetching from the public internet
106-
const promise = (cache[payload] ??= {
107-
serialize: true,
108-
data: fetch(new URL(url, event.url.origin).href).then(async (response) => {
109-
if (!response.ok) {
110-
throw new Error('Prerendered response not found');
111-
}
112-
113-
const prerendered = await response.json();
114-
115-
if (prerendered.type === 'error') {
116-
error(prerendered.status, prerendered.error);
117-
}
118-
119-
return prerendered.result;
120-
})
121-
}).data;
122-
123-
return parse_remote_response(await promise, state.transport);
124-
});
104+
// TODO adapters can provide prerendered data more efficiently than
105+
// fetching from the public internet
106+
const response = await fetch(new URL(url, event.url.origin).href);
107+
108+
if (!response.ok) {
109+
throw new Error('Prerendered response not found');
110+
}
111+
112+
const prerendered = /** @type {RemoteFunctionResponse} */ await response.json();
113+
114+
if (prerendered.type === 'error') {
115+
error(prerendered.status, prerendered.error);
116+
}
117+
118+
return parse_remote_response(prerendered.data, state.transport)._;
125119
} catch {
126120
// not available prerendered, fallback to normal function
127121
}
128122
}
129123

124+
// during a prerender run, the same function might be invoked while rendering
125+
// multiple pages — share the result across the entire run
130126
if (state.prerendering?.remote_responses.has(url)) {
131127
return /** @type {Promise<any>} */ (state.prerendering.remote_responses.get(url));
132128
}
133129

134-
const promise = get_response(__, payload, state, () =>
135-
run_remote_function(event, state, false, () => validate(arg), fn)
136-
);
130+
const promise = run_remote_function(event, state, false, () => validate(arg), fn);
137131

138132
if (state.prerendering) {
139133
state.prerendering.remote_responses.set(url, promise);
@@ -142,7 +136,7 @@ export function prerender(validate_or_fn, fn_or_options, maybe_options) {
142136
const result = await promise;
143137

144138
if (state.prerendering) {
145-
const body = { type: 'result', result: stringify(result, state.transport) };
139+
const body = { type: 'result', data: stringify({ _: result }, state.transport) };
146140
state.prerendering.dependencies.set(url, {
147141
body: JSON.stringify(body),
148142
response: json(body)
@@ -151,7 +145,7 @@ export function prerender(validate_or_fn, fn_or_options, maybe_options) {
151145

152146
// TODO this is missing error/loading/current/status
153147
return result;
154-
})();
148+
});
155149

156150
promise.catch(noop);
157151

0 commit comments

Comments
 (0)